-
Notifications
You must be signed in to change notification settings - Fork 6k
[python] remove imports from models #7905
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
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.
It seems that the version should be 2.4.0~
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.
mvn clean was needed 🤦♂️ thanks for spotting this.
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.
What about the scenario where the discriminator is used?
The lack of import will result in invalid code due to the missing import.
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.
Does this discriminator work if modules can't be imported due to circular dependencies ?
Maybe there is another way to provide paths/data for the discriminator - I don't know, I don't use it. Such imports make more problems for standard usage of generated code.
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.
@kenjones-cisco I dig into it. Finally I've written a test and I also had to fix discriminator to make it working ;)
Now it works without these imports. Child models are created dynamically on the parsing step and models with discriminator don't refer to them - only as a string with name of class.
I still recommend to merge this PR.
Thanks.
@Sarek-Wyona
Sarek-Wyona
Oct 26, 2018
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.
What is the status of this PR? I have a similar fix and was about to submit it before stumbling into this PR.
We are seeing models created with circular imports too which makes the generated code useless unless they are manually commented out!
LGTM! Thanks!
+1
4ba5164 to
949f0e6
Compare
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif 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.0branch for changes related to OpenAPI spec 3.0. Default:master.Description of the PR
It resolves the issue #7541 (circular dependencies in Python models), based on @wy-z work, thanks.
PTAL @taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11)