Adding decimal support for python client generation #19203
Conversation
There was a problem hiding this comment.
When you have time, could you regenerate the sample files as explained in the pull request checklist? It should update several files automatically.
Would you also be able to add a couple of tests to verify that serializing/deserializing objects with decimal work as expected? This would be useful to prevent regression in the future. The "echo client" sample should be a good starting point.
Thanks!
|
@multani thank you for looking. I have ran the build steps now and pushed up the commit. Thank you for the example test I will get some coverage added to the branch this weekend and hit you back up to look. |
|
@multani curious if you can shoot a link to the "echo client" you were referring to? I found this but just wanted to make sure I am in the right spot as there were a couple other spots that looked similar. |
|
I was thinking of this sample: https://github.com/OpenAPITools/openapi-generator/tree/master/samples/client/echo_api/python One example of a past change: |
|
@m-standfuss here is another good starting point to add some tests: you can find other tests for |
| # decimal | ||
| data = Decimal("1.0") | ||
| result = self.api_client.sanitize_for_serialization(data) | ||
| self.assertEquals(result, "1.0") | ||
|
|
There was a problem hiding this comment.
Actually, can you move this into samples/openapi3/client/petstore/python/tests/test_api_client.py instead?
The "pydantic v1" backend will be removed at some point.
There was a problem hiding this comment.
oh good catch, thats on me, fixing now
There was a problem hiding this comment.
@multani @wing328 do i need to worry about the failing steps in the build? They appear to be failures to install poetry for the v1 petstore sample.
Run pipx install --python '/opt/hostedtoolcache/Python/3.7.17/x64/bin/python' poetry
creating virtual environment...
installing poetry...
Fatal error from pip prevented installation. Full pip output in file:
/opt/pipx/logs/cmd_2024-07-22_16.55.46_pip_errors.log
Some possibly relevant errors from pip install:
TypeError: Expected maxsize to be an integer or None
Error installing poetry.
There was a problem hiding this comment.
Hm, I can't retry, but I don't think it's related to your change :)
| import os | ||
| import time |
There was a problem hiding this comment.
Can you revert this change? It has nothing to do with your original PR.
Adding decimal support for python client generation
This PR adds the ability to handle Decimal objects coming in and out of python servers. We "recognize" the open api
decimalfield and map it correctly to the python Decimal class. I consider this a non-breaking change because prior to this if a decimal field was used to generate python client library it just didnt work on decimal fields.#19198
PR checklist
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.
master(upcoming 7.6.0 minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)CC Python community for 👀
@cbornet @tomplus @krjakbrjak @fa0311 @multani