-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hotfix/out of memory error 6394 #7625
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see the comments on the files, and if you could fill out the description in the PR summary as well for when this gets to being merged that would help as well.
@@ -1,18 +1,39 @@ | |||
package io.swagger.codegen.examples; | |||
|
|||
import static io.swagger.models.properties.StringProperty.Format.URI; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these changes to the imports are unnecessary - could you change them back to how they were?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because of the automatically format of IntelliJ. And I have changed the import back.
@@ -179,6 +179,7 @@ private Object resolvePropertyToExample(String propertyName, String mediaType, P | |||
if (innerType != null) { | |||
int arrayLength = null == ((ArrayProperty) property).getMaxItems() ? 2 : ((ArrayProperty) property).getMaxItems(); | |||
if(arrayLength>10000) { | |||
logger.warn("The max item is too large to new Object array and set it to 10000, the previous value is {}", arrayLength); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This warning message is a bit unclear. Could you change it to something like logger.warn("The max items allowed in property {} is too large ({} items), restricting it to 10,000 items", property, arrayLength)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks fine to me. I'm fairly confident the CirleCI failures are unrelated.
Applied the fix listed in a PR on the official swagger-codegen project: swagger-api#7625. The maintainers are dragging their feet on merging it...
Thanks for the PR; a fix, limiting to 10 examples, has been applied in #9553, therefore closing this one. |
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\
.3.0.0
branch for changes related to OpenAPI spec 3.0. Default:master
.Description of the PR
(details of the change, additional tests that have been done, reference to the issue for tracking, etc)
Hotfix for the issue #6394