-
Notifications
You must be signed in to change notification settings - Fork 367
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
Fix Python client "LogCommits" doesn't quote query parameters #8597
base: master
Are you sure you want to change the base?
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.
Hi @joostsijm ,
We are really grateful for any efforts to keep our clients up-to-date, and obviously if the existing generated code has bugs we want to fix those! But at the same time, our users develop long-lived applications that use our published SDKs. Any change to the API must be backwards compatible - or has to wait for a major release. We would obviously not want to wait for a major release. So we do need to understand whether the generated SDK is different.
I am very happy to upgrade the code generator in a way that adds new capabilities to the generated code: that is a minor version bump, and a net gain to our users. I am very guarded about any change to the interfaces of generated code: that is a major version bump, and requires unplanned work for our users.
It appears that there might be some small changes to generated code - mostly in from_*
methods. This is particularly frustrating because the changes I identified appear to apply only to error reporting. So I would like some more context on the changes to generated code. Are my worries founded? Is there some documentation of backwards compatibility of clients generated using different openapi-generator versions? Is there some magic flag I could switch on, that would cause existing code to continue to work in the same way?
THANKS!
|
||
# convert the object into a dict | ||
branch_creation_dict = branch_creation_instance.to_dict() | ||
# create an instance of BranchCreation from a dict | ||
branch_creation_form_dict = branch_creation.from_dict(branch_creation_dict) | ||
branch_creation_from_dict = BranchCreation.from_dict(branch_creation_dict) |
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.
Is this (and obviously related changes in every API...) a required change to make to code? That is, will the previous code continue to work?
We provide a backwards compatibility guarantee for users of our published APIs. We unfortunately cannot accept a change if it breaks backwards compatibility.
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 to me that this is just a correction on the documentation, the previous implementation would error because branch_creation
is an undefined name. The new version also corresponds better to the annotation create an instance of BranchCreation from a dict
return _dict | ||
|
||
@classmethod | ||
def from_dict(cls, obj: dict) -> BranchCreation: | ||
def from_dict(cls, obj: Optional[Dict[str, Any]]) -> Optional[Self]: |
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.
The change is return type appears to break backwards compatibility: AFAIU the current library raises ValidationError for an invalid dict but never returns None.
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.
The typing has changed, the implementation hasn't: on line 68 (old) / 84 (new), there is None
returned. So this is a correction on the typing and no behavioural change.
@joostsijm The image change creates also changes in the Java SDK. Please build the SDK client and push the changes as well |
Closes #8558
Change Description
Change openapi-generator-cli docker image to OpenAPITools, because theerverse version is not up-to-date.
Bug Fix
See #8558
Testing Details
No additional test added to verify resolve conflict, as this is already covered in the openapi pull request
Breaking Change?
Python version required is updated to 3.8 from 3.7.
Additional info
Line change which resolve problem in #8558
Contact Details
[email protected]