Skip to content

[RFC] Major breaking changes in an upcoming release, python client generation #1943

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

Closed
spacether opened this issue Nov 2, 2022 · 6 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@spacether
Copy link

spacether commented Nov 2, 2022

Request for comments (rfc):

Hi Kubernetes Python client developers, this is a request for comment issue. Please leave comments if you have any opinion/suggestion. Thanks!

What happened:
A majority of the code base is generated code. This repo is switching from using openapi-generator v4.3.0 to v6.2.1. For more background:

#1887

This change updates the generated python client code from 4.3.0, March 2020 to 6.2.1, November 2022
The newer python client code

#1887
This new code has some breaking changes which will require code that uses the client to be updated.
The breaking changes are:

  1. [MAJOR] This generator uses spec case for all (object) property names and parameter names.
    • So if the spec has a property name like camelCase, it will use camelCase rather than camel_case
    • So you will need to update how you input and read properties to use spec case
  2. [MAJOR] Endpoint parameters are stored in dictionaries to prevent collisions
    • So you will need to update all of your endpoint calls that have query/path/header/cookie params to pass that parameter data in query_params, header_params, path_params, cookie_params named argument inputs with values of type dict
  3. [MAJOR] Endpoint responses now include the original response, the deserialized response body, and (todo)the deserialized headers
    • So you will need to update your code to use response.body to access deserialized data
  4. [MAJOR] All validated data is instantiated in an instance that subclasses all validated Schema classes and Decimal/str/list/tuple/frozendict/NoneClass/BoolClass/bytes/io.FileIO
    • This means that you can use isinstance to check if a payload validated against a schema class
    • This means that no data will be of type None/True/False
      • ingested None will subclass NoneClass
      • ingested True will subclass BoolClass
      • ingested False will subclass BoolClass
      • So if you need to check is True/False/None, instead use instance.is_true_oapg()/.is_false_oapg()/.is_none_oapg()
      • This was done
        • So one can run expensive validation once at instantiation time and later validation checks can use isinstance rather than re-running expensive validation
        • Because there are json schema tests that check that 0 != False and 1 != True
  5. [MAJOR] All validated class instances are immutable except for ones based on io.File
    • This is because if properties were changed after validation, that validation would no longer apply
    • So no changing values or property values after a class has been instantiated
  6. [MAJOR] String + Number types with formats
    • String type data is stored as a string and if you need to access types based on its format like date,
      date-time, uuid, number etc then you will need to use accessor functions on the instance
    • type string + format: See .as_date_oapg, .as_datetime_oapg, .as_decimal_oapg, .as_uuid_oapg
    • type number + format: See .as_float_oapg, .as_int_oapg
    • this was done because openapi/json-schema defines constraints. string data may be type string with no format
      keyword in one schema, and include a format constraint in another schema
    • So if you need to access a string format based type, use as_date_oapg/as_datetime_oapg/as_decimal_oapg/as_uuid_oapg
    • So if you need to access a number format based type, use as_int_oapg/as_float_oapg
  7. [MAJOR] Property access on AnyType(type unset) or object(dict) schemas
    • Only required keys with valid python names are properties like .someProp and have type hints
    • All optional keys may not exist, so properties are not defined for them
    • One can access optional values with dict_instance['optionalProp'] and KeyError will be raised if it does not exist
    • Use get_item_oapg if you need a way to always get a value whether or not the key exists
      • If the key does not exist, schemas.unset is returned from calling dict_instance.get_item_oapg('optionalProp')
      • All required and optional keys have type hints for this method, and @typing.overload is used
      • A type hint is also generated for additionalProperties accessed using this method
    • So you will need to update you code to use some_instance['optionalProp'] to access optional property
      and additionalProperty values
  8. [MAJOR] The location of the api classes has changed
    • Api classes are located in kubernetes.client.apis.tags.some_api
    • This change was made to eliminate redundant code generation
    • Legacy generators generated the same endpoint twice if it had > 1 tag on it
    • This generator defines an endpoint in one class, then inherits that class to generate
      apis by tags and by paths
    • This change reduces code and allows quicker run time if you use the path apis
      • path apis are at your_package.apis.paths.some_path
    • Those apis will only load their needed models, which is less to load than all of the resources needed in a tag api
    • So you will need to update your import paths to the api classes
  9. [MAJOR] Python min version changed from 3.6 to 3.7

What versions are affected?:

TBD if this is accepted, then an impacted version would be decided.

@spacether spacether added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 2, 2022
@tomplus
Copy link
Member

tomplus commented Nov 2, 2022

Thanks for your contribution 👍

Definitely it's worth to use the latest version of openapi-generator but from our customers' perspective (people who use this library) each breaking change is always a headache. We should consider adding proxy classes/modules to allow to use old code with deprecation warnings. We used to have a similar problem with renaming modules and we added this:
https://github.com/kubernetes-client/python/blob/master/kubernetes/client/apis/__init__.py

Also adding some examples to description of change would be useful. Some of them are not clear for me, for example: "So you will need to update how you pass data in to endpoints". Is it for all endpoints? Is my code affected? How can I fix it?

@spacether
Copy link
Author

Thanks for the feedback @tomplus
I will work on updating examples and e2e tests.
Just now I updated that statement to read - So you will need to update all of your endpoint calls that have query/path/header/cookie params to pass that parameter data in query_params, header_params, path_params, cookie_params named argument inputs with values of type dict
What other points need clarification?

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 12, 2023
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 14, 2023
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

4 participants