Fix Python client "LogCommits" doesn't quote query parameters #8597
Conversation
|
|
There was a problem hiding this comment.
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!
| 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.
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.
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
|
|
||
| @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.
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.
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 |
joostsijm
changed the title
|
Hi @joostsijm, I wanted to let you know that I took your proposed fix for openapi-generator, applied it to version v7.0.1, and then pushed this updated version as v7.0.1.1 in our Treeverse Docker repository. Additionally, created a pull request at treeverse/lakeFS#8652 to incorporate these changes into lakeFS. We hope to get this merged and included in the upcoming release soon. |
|
Changes where merged into master and will be released as part of the next lakeFS release (>v1.49.1) |
6 participants
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
joost.sijm@nelen-schuurmans.nl