-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[Python] fix numeric enum in python flask, aiohttp #5019
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,8 +89,8 @@ class {{classname}}(Model): | |
:type {{name}}: {{dataType}} | ||
""" | ||
{{#isEnum}} | ||
allowed_values = [{{#allowableValues}}{{#values}}"{{{this}}}"{{^-last}}, {{/-last}}{{/values}}{{/allowableValues}}] | ||
{{#isContainer}} | ||
allowed_values = [{{#isNullable}}None,{{/isNullable}}{{#allowableValues}}{{#values}}{{#items.isString}}"{{/items.isString}}{{{this}}}{{#items.isString}}"{{/items.isString}}{{^-last}}, {{/-last}}{{/values}}{{/allowableValues}}] # noqa: E501 | ||
{{#isListContainer}} | ||
if not set({{{name}}}).issubset(set(allowed_values)): | ||
raise ValueError( | ||
|
@@ -109,6 +109,7 @@ class {{classname}}(Model): | |
{{/isMapContainer}} | ||
{{/isContainer}} | ||
{{^isContainer}} | ||
allowed_values = [{{#isNullable}}None,{{/isNullable}}{{#allowableValues}}{{#values}}{{#isString}}"{{/isString}}{{{this}}}{{#isString}}"{{/isString}}{{^-last}}, {{/-last}}{{/values}}{{/allowableValues}}] # noqa: E501 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are tying to minimize the usage of flake8 E501 and other flake8 opt out lines. |
||
if {{{name}}} not in allowed_values: | ||
raise ValueError( | ||
"Invalid value for `{{{name}}}` ({0}), must be one of {1}" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,15 +25,17 @@ class {{classname}}(Model): | |
"""NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). | ||
|
||
Do not edit the class manually. | ||
"""{{#allowableValues}} | ||
""" | ||
{{#allowableValues}} | ||
|
||
""" | ||
allowed enum values | ||
""" | ||
{{#enumVars}} | ||
{{name}} = {{{value}}}{{^-last}} | ||
{{/-last}} | ||
{{/enumVars}}{{/allowableValues}} | ||
{{/enumVars}} | ||
{{/allowableValues}} | ||
|
||
def __init__(self{{#vars}}, {{name}}={{#defaultValue}}{{{defaultValue}}}{{/defaultValue}}{{^defaultValue}}None{{/defaultValue}}{{/vars}}): # noqa: E501 | ||
"""{{classname}} - a model defined in OpenAPI | ||
|
@@ -96,8 +98,8 @@ class {{classname}}(Model): | |
:type {{name}}: {{dataType}} | ||
""" | ||
{{#isEnum}} | ||
allowed_values = [{{#allowableValues}}{{#values}}"{{{this}}}"{{^-last}}, {{/-last}}{{/values}}{{/allowableValues}}] # noqa: E501 | ||
{{#isContainer}} | ||
allowed_values = [{{#isNullable}}None,{{/isNullable}}{{#allowableValues}}{{#values}}{{#items.isString}}"{{/items.isString}}{{{this}}}{{#items.isString}}"{{/items.isString}}{{^-last}}, {{/-last}}{{/values}}{{/allowableValues}}] # noqa: E501 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are tying to minimize the usage of flake8 E501 and other flake8 opt out lines. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's do this in a separate PR instead. I'll open an issue to track it to see if anyone has the time to improve it. (this PR simply plots the fix from the python client template) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you suggesting that we remove the E501s in a separate PR? That sounds good. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created #5030 for tracking. |
||
{{#isListContainer}} | ||
if not set({{{name}}}).issubset(set(allowed_values)): | ||
raise ValueError( | ||
|
@@ -116,6 +118,7 @@ class {{classname}}(Model): | |
{{/isMapContainer}} | ||
{{/isContainer}} | ||
{{^isContainer}} | ||
allowed_values = [{{#isNullable}}None,{{/isNullable}}{{#allowableValues}}{{#values}}{{#isString}}"{{/isString}}{{{this}}}{{#isString}}"{{/isString}}{{^-last}}, {{/-last}}{{/values}}{{/allowableValues}}] # noqa: E501 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are tying to minimize the usage of flake8 E501 and other flake8 opt out lines. |
||
if {{{name}}} not in allowed_values: | ||
raise ValueError( | ||
"Invalid value for `{{{name}}}` ({0}), must be one of {1}" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ paths: | |
required: true | ||
x-body-name: body | ||
responses: | ||
405: | ||
"405": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These spec updates are unexpected in this pull review because we did not edit the spec file.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added to ensure-up-to-date. These scripts are using petstore.yaml (original) but not the fake petstore spec (which has numeric enums) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a model with numeric enums in petstore.yaml? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can update the script to use the fake petstore spec instead (petstore.yaml shouldn't be updated) When a generator gets mature, we will use fake petstore spec instead of petstore.yaml/json as the generator should be able to handle those edge cases in fake petstore spec. I'm not sure if these generators can handle the edge cases in fake petstore spec. |
||
content: {} | ||
description: Invalid input | ||
security: | ||
|
@@ -58,13 +58,13 @@ paths: | |
required: true | ||
x-body-name: body | ||
responses: | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid ID supplied | ||
404: | ||
"404": | ||
content: {} | ||
description: Pet not found | ||
405: | ||
"405": | ||
content: {} | ||
description: Validation exception | ||
security: | ||
|
@@ -97,7 +97,7 @@ paths: | |
type: array | ||
style: form | ||
responses: | ||
200: | ||
"200": | ||
content: | ||
application/xml: | ||
schema: | ||
|
@@ -110,7 +110,7 @@ paths: | |
$ref: '#/components/schemas/Pet' | ||
type: array | ||
description: successful operation | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid status value | ||
security: | ||
|
@@ -139,7 +139,7 @@ paths: | |
type: array | ||
style: form | ||
responses: | ||
200: | ||
"200": | ||
content: | ||
application/xml: | ||
schema: | ||
|
@@ -152,7 +152,7 @@ paths: | |
$ref: '#/components/schemas/Pet' | ||
type: array | ||
description: successful operation | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid tag value | ||
security: | ||
|
@@ -179,7 +179,7 @@ paths: | |
format: int64 | ||
type: integer | ||
responses: | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid pet value | ||
security: | ||
|
@@ -202,7 +202,7 @@ paths: | |
format: int64 | ||
type: integer | ||
responses: | ||
200: | ||
"200": | ||
content: | ||
application/xml: | ||
schema: | ||
|
@@ -211,10 +211,10 @@ paths: | |
schema: | ||
$ref: '#/components/schemas/Pet' | ||
description: successful operation | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid ID supplied | ||
404: | ||
"404": | ||
content: {} | ||
description: Pet not found | ||
security: | ||
|
@@ -246,7 +246,7 @@ paths: | |
type: string | ||
x-body-name: body | ||
responses: | ||
405: | ||
"405": | ||
content: {} | ||
description: Invalid input | ||
security: | ||
|
@@ -283,7 +283,7 @@ paths: | |
type: string | ||
x-body-name: body | ||
responses: | ||
200: | ||
"200": | ||
content: | ||
application/json: | ||
schema: | ||
|
@@ -303,7 +303,7 @@ paths: | |
description: Returns a map of status codes to quantities | ||
operationId: get_inventory | ||
responses: | ||
200: | ||
"200": | ||
content: | ||
application/json: | ||
schema: | ||
|
@@ -330,7 +330,7 @@ paths: | |
required: true | ||
x-body-name: body | ||
responses: | ||
200: | ||
"200": | ||
content: | ||
application/xml: | ||
schema: | ||
|
@@ -339,7 +339,7 @@ paths: | |
schema: | ||
$ref: '#/components/schemas/Order' | ||
description: successful operation | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid Order | ||
summary: Place an order for a pet | ||
|
@@ -360,10 +360,10 @@ paths: | |
schema: | ||
type: string | ||
responses: | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid ID supplied | ||
404: | ||
"404": | ||
content: {} | ||
description: Order not found | ||
summary: Delete purchase order by ID | ||
|
@@ -385,7 +385,7 @@ paths: | |
minimum: 1 | ||
type: integer | ||
responses: | ||
200: | ||
"200": | ||
content: | ||
application/xml: | ||
schema: | ||
|
@@ -394,10 +394,10 @@ paths: | |
schema: | ||
$ref: '#/components/schemas/Order' | ||
description: successful operation | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid ID supplied | ||
404: | ||
"404": | ||
content: {} | ||
description: Order not found | ||
summary: Find purchase order by ID | ||
|
@@ -486,7 +486,7 @@ paths: | |
schema: | ||
type: string | ||
responses: | ||
200: | ||
"200": | ||
content: | ||
application/xml: | ||
schema: | ||
|
@@ -506,7 +506,7 @@ paths: | |
schema: | ||
format: date-time | ||
type: string | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid username/password supplied | ||
summary: Logs user into the system | ||
|
@@ -536,10 +536,10 @@ paths: | |
schema: | ||
type: string | ||
responses: | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid username supplied | ||
404: | ||
"404": | ||
content: {} | ||
description: User not found | ||
summary: Delete user | ||
|
@@ -556,7 +556,7 @@ paths: | |
schema: | ||
type: string | ||
responses: | ||
200: | ||
"200": | ||
content: | ||
application/xml: | ||
schema: | ||
|
@@ -565,10 +565,10 @@ paths: | |
schema: | ||
$ref: '#/components/schemas/User' | ||
description: successful operation | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid username supplied | ||
404: | ||
"404": | ||
content: {} | ||
description: User not found | ||
summary: Get user by user name | ||
|
@@ -594,10 +594,10 @@ paths: | |
required: true | ||
x-body-name: body | ||
responses: | ||
400: | ||
"400": | ||
content: {} | ||
description: Invalid user supplied | ||
404: | ||
"404": | ||
content: {} | ||
description: User not found | ||
summary: Updated user | ||
|
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.
We are tying to minimize the usage of flake8 E501 and other flake8 opt out lines.
Can you spread this to a multiline statement so we won't need # noqa: E501?