-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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-experimental] generate model if type != object if enums/validations exist #2757
[python-experimental] generate model if type != object if enums/validations exist #2757
Conversation
modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java
Outdated
Show resolved
Hide resolved
5ad7737
to
8541d2f
Compare
All tests pass, ready for final review @wing328 |
6052c0e
to
fdfc6dd
Compare
@spacether thanks for the PR. Let me review later today |
aa6ae98
to
49e4124
Compare
The travis ci failure is because the Cat and Dog models somehow invoked two templates to generate instances of ModelNormal. I will look into this. This is a new problem and was not happening before my 49e4124 rebase. |
@wing328 I fixed the bug. All tests now pass. This PR is ready for review. |
Hi @wing328 |
@spacether Yup, very busy as usual. Thanks again for the PR and keeping it up-to-date. I'll review over the weekend and let you know if I've any question. |
cc @OpenAPITools/generator-core-team since the change covers default codegen as well. |
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.
I'm not familiar with Python but reviewed the changes on DefaultCodegen.
modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
Outdated
Show resolved
Hide resolved
modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java
Outdated
Show resolved
Hide resolved
c3d6bd2
to
c427de6
Compare
modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java
Outdated
Show resolved
Hide resolved
.../openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java
Outdated
Show resolved
Hide resolved
I still need to tweak the python generator so it handles the below models better: |
If public class StringBooleanMap extends HashMap<String, Boolean> { If
The above explanation applies to AnimalFarm as well:
If you have any questions about "alias", please let me know and I'll try my best to explain. |
Thanks for that explanation. I think that StringBooleanMap is being generated correctly in python. |
Using the command line argument to stop generation of alias models requires removing files from 29 generator sample folders. I am working through them. |
@spacether you mean this change will generate additional files and require the switch (generateAliasAsModel) to stop those files from being generated? |
@wing328 nope. So far this is file removal. |
8f12836
to
03bd6ed
Compare
@wing328 all better and ready for reviews. |
7bbd5aa
to
f886098
Compare
protected void handleMethodResponse(Operation operation, | ||
Map<String, Schema> schemas, | ||
CodegenOperation op, | ||
ApiResponse methodResponse) { |
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 this code was moved and not changed in any way
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.
Yup, noticed that too.
if (modelTemplate != null && modelTemplate.containsKey("model")) { | ||
CodegenModel m = (CodegenModel) modelTemplate.get("model"); | ||
if (m.isAlias) { | ||
if (m.isAlias && !ModelUtils.isGenerateAliasAsModel()) { |
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.
@spacether without && !ModelUtils.isGenerateAliasAsModel()
it won't work in your case right?
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.
That's right
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.
FYI. I've filed #3951 as && !ModelUtils.isGenerateAliasAsModel()
impacts all other generators as well.
f886098
to
762bc06
Compare
samples/client/petstore/python-experimental/openapi_client/api_client.py
Outdated
Show resolved
Hide resolved
...erator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java
Show resolved
Hide resolved
...erator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java
Show resolved
Hide resolved
@wing328 all feedback has been addressed. This PR is ready for another review. |
Tested the generateAliasAsModel option and worked as expected. |
* master: (207 commits) Add missing enum processing in C++ codegen, already present for Qt5 (#3986) [C++] [Pistache] Removed deprecated warnings (#3985) [C++][Pistache] Simplified model template (#3417) add go oas3 petstore to ensure up-to-date (#3979) replace gitter with slack in the doc (#3977) Fix wrong variable name in LessThan and LessThanOrEqual asserts (#3971) #3957 - Removed hardcoded baseUrl (#3964) Regenerate go openapi3 samples (#3975) [rust] Make it easier to test rust client generator (#3543) Fix issue3635 (#3948) add gradle repository (#3867) [java] allow to use setArtifactVersion() programmatically (#3907) Add a link to DevRelCon SF 2019 (#3961) Add a link to a medium blog post (#3960) update maven-compiler-plugin version (#3956) fix generateAliasAsModels in default generator (#3951) Implement BigDecimal to Decimal in swift4 for currency data as type=string format=number (#3910) Add F# Functions server generator (#3933) [python-experimental] generate model if type != object if enums/validations exist (#2757) [scala] add [date-time] field to codegen unit test (#3939) ...
…ations exist (OpenAPITools#2757) * Python-experimental adds model_utils module, refactors python api class * Fixes python-experimental so the sample sare generated in the petstore_api folder * FIxes python samples tests * Updates python and python-experimental tests * Fixes python-experimental tests * Adds newlines back to python templates + samples * Reverts files with newline tweaks back to master branch versions * Fixes indentation errors in python-experimental api_client * Removes unused files * Python files now generated in correct folders * Adds logging when the user tries to set generateAliasAsModel in python-experimental * Fixes typo
wow, why not to make this behavior tweakable? Also, how client users should import enums right now? |
What way do you want it to be tweakable?
Then we need to produce the StringWithValidation model to contain the StringWithValidation validation logic. For now, about your enums question, yes using |
I just want to mention enums part of PR. Also, right now enum client serialization is broken and produce Also, in some cases you want to have behavior described there #6828 (as does current generator). Can't find property, how to disable enums value check while initializing class during de-serialization on |
Enum serialization issueHi, can you create a new issue describing how enums are not working with a sample spec? Enum checking allowed valuesRight now there is no way to disable checking allowed values per: openapi-generator/samples/openapi3/client/petstore/python-experimental/petstore_api/model_utils.py Line 145 in 2743242
Why would you want to disable it? What if the server sends an invalid value because your client spec document is out of date? Enum Constants
Would your needs for a static enum be met with Enum classes like:
where the value that you send or receive would be |
I think my problem do not related to enums directly, but it is lack of static constants description in openapi. Thus, if I have integer field, that have set of 100 predefined values, i want to have:
currently, for java client, i use 2 cycle generation. on first run, I produce from modified enum template file with constants. On a second run I use type-mappings to replaces openapi enum model with primitive values. btw, will try to provide full same of wrong enum serialization tomorrow |
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
,./bin/openapi3/{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\
.master
,. Default:3.4.x
,4.0.x
master
.@taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11) @tomplus (2018/10) @Jyhess (2019/01)
Description of the PR
This PR makes it so models whose type != "object" are generated in python if the model has validations or enums. Those models are then used for serialization and deserialization when communicating with an API.
Why is this necessary?
Description of Updates:
api_name.endpoint_name.validations[('var_name',)] = {'inclusive_maximum': 100}
model_class.validations[('var_name',)] = {'inclusive_maximum': 100}
Note: merging this PR will close this issue: #1991