-
Notifications
You must be signed in to change notification settings - Fork 6k
[Python] Nullable bugfix in model.mustache #10906
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
Before bugfix: a required and nullable attribute set to null/None causes a value error when received by the client. After bugfix: This error is only caused for properties being required and not nullable General info: The python client cannot distinguish between unset attributes (value=None) and attributes set to null (value=None)
|
Hi, any update on this? Do you know when it will be merged? Thank you (: |
Sorry, I would love to see it merged, but do not know get the maintainers to do it. |
I don't know if it may be relevant, but from the PR preview it looks like there are missing tasks required to close it Any idea on how to fix this? I guess all 4 must be checked before the PR can be merged |
The tasks are one for each checkbox in the PR description. They are not necessary to get it merged, see I guess the issue is rather that @kenjones-cisco did not find the time to review and he's responsible for the python part: https://github.com/swagger-api/swagger-codegen/#swagger-codegen-technical-committee |
|
Any progress here? Does something need to be changed in this PR or not to be good to merge and fix this problem? |
I guess the problem is that @kenjones-cisco is still not active. Perhaps you could update us here @HugoMario, as you have been the last to update this file, according to the git blame. |
|
I just ran into this problem as well, would be great if we can get this merged and released. I will have to work around it by just removing all the required attributes from the JSON file before generating code, which isn't ideal. Here is the jq command I used to do that in case anyone else wants to do the same workaround: Not sure who can approve but just from the recent people who have committed code, can anyone help? @frantuma, @HugoMario, @gracekarina |
|
How can we get this change merged? I can't believe it's been there for more than a year. |
|
@HugoMario @keithbsb this seems like a pretty easy change for any maintainer to move along. Thoughts on helping us out? |
|
We also ran into this bug. Maybe @frantuma could unblock us all here? He seems to be the most recent active maintainer 😁 |
|
I'm also running into this issue. |
|
I´m also running in this issue. |
|
Can this be please merged? It is perfectly fine to have required fields which are nullable. Having a key marked as required just means it will be present in the output, it doesn't make any guarantees about the value, and null is a perfectly valid value. |
…r-api/swagger-codegen#10906). Need to reorder the plugins because this requires Antrun before Swagger-Codegen but also Surefire has a position constraint relative to Antrun.
…ger-codegen -l python (swagger-api/swagger-codegen#10906).
|
I need to create a new PR request because there were problems building the PR from the fork. New PR is #12630. The author of change is still @MalteEbner. Thank you very much for your contribution and patience. Fix should be released in next week. |
|
#12630 was merged into master, and is now part of release 2.4.49 two days ago! Thank you @daniel-kmiecik! 🥳 |

Description of the PR
Before bugfix: a required and nullable attribute set to null/None causes a ValueError when received by the client.
After bugfix: This error is only caused for properties being required and not nullable
General info: The python client cannot distinguish between unset attributes (value=None) and attributes set to null (value=None)
Issue to solve with a good description of the bug:
closes: #10535
closes: #8710
PR checklist
Performed tests:
Tested with own api-specs and the swagger-cli: