Support dict[...] syntax in ApiClient.__deserialize #2467
Conversation
…upport) Adds post-gen patch; aligns with list[...] and pydantic adapters; no behavior change for legacy types. Fixes kubernetes-client#2463
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
do-not-merge/invalid-commit-message
|
Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages. The list of commits with invalid commit messages:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mohsinm-dev The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/M
vit-zikmund
left a comment
vit-zikmund
left a comment
There was a problem hiding this comment.
Hello Mohsin, Thanks a lot for picking up my issue!
I'd like to ask two questions. One is in the review comments and the other is: What's the change needed to be done on the openapi generator side? Do you know?
Thanks a lot!
PS: There are some failed automatic checks which add the do-not-merge labels. I'm not sure how the review process works in this repo, but it surely is understaffed (as usual) and a human reviewer won't IMHO pay attention to this PR until it's labeled lgtm (= all the automatic PR checks are resolved).
| if klass.startswith('dict[') and klass.endswith(']'): | ||
| # Parse dict[key_type, value_type] respecting nested brackets | ||
| inner = klass[len('dict['):-1] | ||
| bracket_depth = 0 | ||
| comma_pos = -1 | ||
| for i, char in enumerate(inner): | ||
| if char in '([{': | ||
| bracket_depth += 1 | ||
| elif char in ')]}': | ||
| bracket_depth -= 1 | ||
| elif char == ',' and bracket_depth == 0: | ||
| comma_pos = i | ||
| break | ||
|
|
||
| if comma_pos != -1: | ||
| value_type = inner[comma_pos + 1:].strip() |
There was a problem hiding this comment.
Why wasn't reusing the usual regex approach (used for dict() satisfactory?
3 participants
Summary
dict[str, str]syntax inApiClient.__deserialize()dict(str, str)syntaxopenapi_typesChanges
scripts/api_client_dict_syntax.diffscripts/update-client.shto apply patch during client generationTest plan
test_deserialize_dict_syntax_compatibilitypassesdict(str, str)syntax continues to workdict[str, str]syntax works correctlydict[str, dict[str, str]]syntax worksFixes #2463