Don't drop null fields from request bodies #19068
Conversation
|
ping python tech committee members: |
| elif isinstance(obj, dict): | ||
| obj_dict = obj | ||
| elif hasattr(obj, "model_dump") and callable(obj.model_dump): |
There was a problem hiding this comment.
unfortunately the way to_dict/from_dict integrate this breaks in the face of the from/to dict layer the codegen still ads
as all from_dict call convert missing fields to None, pydantic has no chance to distinguish passed in from missing
There was a problem hiding this comment.
If both the serialization and deserialization logic has issues, then do they need to be solved at the same time? That is, if this merge request successfully improves upon serialization behavior, then can we consider improving deserialization as a separate problem?
If we do need to improve both serialization and deserialization logic at the same time, then do you have a suggestion as for how to do that?
|
Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors. Let me know if you need help fixing it. |
Hmm, I don't see this issue. The commits in the "Commits" tab point to my account. And per
|
|
@Ichimonji10 would you be able to also add one or more tests to test the new behavior and prevent regressions? |
The python API bindings drop null request field from HTTP request
bodies. This is especially problematic for HTTP PUT and PATCH requests,
where null values are semantically important.
Change the behavior of the bindings:
```python
# before
model_dump(by_alias=True, exclude=excluded_fields, exclude_none=True)
# after
model_dump(by_alias=True, exclude_unset=True)
```
The effect of this change is to make the API send data to the remote
server that hews more closely to the original model. This is important
when making HTTP PUT and PATCH requests:
```python
# model describing a movie you've watched
class LogEntry(BaseModel):
name: str
rating: int | None = None
# create log entry
log_entry = LogEntry(name="Blade", rating=5)
api_client.put_log_entry(1, log_entry)
# clear rating; PUT {"name": "Blade", "rating": null}
log_entry = LogEntry(name="Blade", rating=None)
api_client.put_log_entry(1, log_entry)
# clear rating; PATCH {"rating": null}
log_entry = LogEntry.model_construct(rating=None)
api_client.patch_log_entry(1, log_entry)
```
Before this change, the API client would omit `"rating": null` from
request bodies.
Fix: OpenAPITools#19067
Done with: ```bash ./bin/generate-samples.sh ./bin/configs/python*.yaml ```
Ichimonji10
force-pushed
the
dont-drop-null
branch
from
f623679
to
7bd49d3
Compare
|
I see that this test assertion is failing: # samples/client/echo_api/python/tests/test_manual.py
self.assertEqual(api_response, "{'name': 'testing', 'photoUrls': ['http://1', 'http://2']}")I can fix that test assertion easily enough, but I'm not quite sure how to go about it. There are three |
|
@Ichimonji10 |
The python API bindings drop null request field from HTTP request
bodies. This is especially problematic for HTTP PUT and PATCH requests,
where null values are semantically important.
Change the behavior of the bindings:
The effect of this change is to make the API send data to the remote
server that hews more closely to the original model. This is important
when making HTTP PUT and PATCH requests:
Before this change, the API client would omit
"rating": nullfromrequest bodies.
Fix: #19067