Skip to content

[Python] Add enum support when building default values for model properties when using $ref#18796

Merged
wing328 merged 6 commits into
OpenAPITools:masterfrom
welshm:welshm.add_enum_support_python_codegen
Jun 8, 2024
Merged

[Python] Add enum support when building default values for model properties when using $ref#18796
wing328 merged 6 commits into
OpenAPITools:masterfrom
welshm:welshm.add_enum_support_python_codegen

Conversation

@welshm

@welshm welshm commented May 30, 2024

Copy link
Copy Markdown
Contributor

Add in better processing for enums in Python when they are a $ref

  • Currently, doesn't properly populate default values so when models are de/serialized, the enum value is dropped.
  • Adds support for checking the $ref when getting the default value for a Schema
  • Updates how enum variable names are referenced
    • Update DefaultCodegen.java with public String toEnumDefaultValue(CodegenProperty property, String value)
    • Allows checking if the property is an enum reference - and if so, handle it as a first class enum model in Python
    • Otherwise, use the inline variable value (as it does today)

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh ./bin/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    
    (For Windows users, please run the script in Git BASH)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.6.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@welshm welshm changed the title Add enum support when building default values for model properties Add enum support when building default values for model properties when using $ref May 30, 2024
@welshm
welshm marked this pull request as ready for review May 30, 2024 13:42
@welshm

welshm commented May 30, 2024

Copy link
Copy Markdown
Contributor Author

@wing328 @spacether @krjakbrjak I don't see Flask/Connexion as being supported (just FastAPI) but this impacts all of Python.

@welshm welshm changed the title Add enum support when building default values for model properties when using $ref [Python] Add enum support when building default values for model properties when using $ref May 30, 2024
}
if (enumName != null) {
var.defaultValue = toEnumDefaultValue(enumName, var.datatypeWithEnum);
var.defaultValue = toEnumDefaultValue(var, enumName);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi @OpenAPITools/generator-core-team

outputDir: samples/server/petstore/python-flask
outputDir: samples/server/petstore/2_0/python-flask
inputSpec: modules/openapi-generator/src/test/resources/2_0/python-flask/petstore.yaml
templateDir: modules/openapi-generator/src/main/resources/python-flask

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't test 2.0 spec so no need to keep this sample folder and yaml config file

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated - thanks!

@welshm
welshm requested a review from jimschubert as a code owner June 5, 2024 14:00
@welshm

welshm commented Jun 5, 2024

Copy link
Copy Markdown
Contributor Author

Something went wrong doing the rebase - will fix

@welshm

welshm commented Jun 5, 2024

Copy link
Copy Markdown
Contributor Author

The code is now correct, but the commit history is a bit mangled. I can open another PR if that's a problem.

Let me know! @wing328

@welshm
welshm requested a review from wing328 June 6, 2024 14:46
@welshm
welshm force-pushed the welshm.add_enum_support_python_codegen branch from 23300fb to 1d8dd73 Compare June 6, 2024 14:51
@wing328

wing328 commented Jun 8, 2024

Copy link
Copy Markdown
Member

thanks for the PR. the commit history looks ok as we will squash the commits during merge.

tested python client with the new test schemas and the result is good: #18882

@wing328
wing328 merged commit a5a9958 into OpenAPITools:master Jun 8, 2024
@wing328 wing328 added this to the 7.7.0 milestone Jun 8, 2024
@wing328

wing328 commented Jun 8, 2024

Copy link
Copy Markdown
Member

fyi. filed #18883 to add the tests in python client as well

@wing328

wing328 commented Jun 12, 2024

Copy link
Copy Markdown
Member

@welshm when you've time, can you please PM me via Slack? thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants