Add a new Python client generator - python-nextgen

[wing328]

To test the new python-nextgen generator, please run the following:

git clone --depth 1 -b python-nextgen https://github.com/openapitools/openapi-generator
cd openapi-generator
./mvnw clean package -DskipTests -Dmaven.javadoc.skip=true
java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g python-nextgen -i https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/3_0/petstore.yaml --additional-properties packageName=openapi_client -o /tmp/openapi_client/

Please give it a try and let us know if you've any feedback by replying to this PR.

Welcome PR for improvements, bug fixes, etc targeting this branch python-nextgen and I'll review accordingly.

This new generator is still in beta and maybe subject to breaking changes without further notice.

(please ignore the python-legacy test for the time being)

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

cc @OpenAPITools/generator-core-team

[tomplus]

I did some tests and I like this approach. It gives us new features like support for type hints and it is still compatible with older generator (even with python-legacy with aiohttp). Thanks 👍

[chludwig-haufe]

I have mostly a user perspective. From what I saw, the generated code looked easy to use; so I like the approach.

However, as it stands, the generator didn't generate valid code for the API I tested it on, cf. my inline comments.

[chludwig-haufe]

The API spec I tried the generator on defines responses as follows:

responses:
        200:
          description: Ok
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/SearchProfile'
        4XX:
          $ref: '#/components/responses/4XX'
        default:
          $ref: '#/components/responses/InternalError'

The generated snippet looks like this:

_response_types_map = {
            200: "List[SearchProfile]",
            4XX: "ErrorResponse",
        }

Obviously, 4XX is not a valid literal. And the default response has been omitted.

[wing328]

i've used string as the key to avoid compilation error for the time being.

still need to work on better handling of response status code using wild card.

will do it with separate PR after merging this one.

[wing328]

@chludwig-haufe thanks for the feedback. will try to incorporate those this week.

[awildturtok]

I ran this on a small spec of mine and generally like the generated code as it is surprisingly readable. On top, the generation is blazing fast. Particularly like the useOneOfDiscriminatorLookup as it very directly adresses my issue.

[awildturtok]

The following component produces a broken from_json for requiredTime.

query-status:
    type: object
    properties:
      requiredTime:
        oneOf:
          - type: integer
          - type: 'null'

from __future__ import annotations
from inspect import getfullargspec
import pprint
import json
import re  # noqa: F401

from typing import Any, List, Optional
from pydantic import BaseModel, Field, StrictInt, StrictStr, ValidationError, validator
from typing import Any, List
from pydantic import StrictStr, Field

QUERYSTATUSREQUIREDTIME_ONE_OF_SCHEMAS = ["int", "none_type"]

class QueryStatusRequiredTime(BaseModel):
 
    # data type: int
    __oneof_schema_1: Optional[StrictInt] = None
    actual_instance: Any
    one_of_schemas: List[str] = Field(QUERYSTATUSREQUIREDTIME_ONE_OF_SCHEMAS, const=True)

    class Config:
        validate_assignment = True

    @validator('actual_instance')
    def actual_instance_must_validate_oneof(cls, v):
        error_messages = []
        match = 0
        # validate data type: int
        if type(v) is not int:
            error_messages.append(f"Error! Input type `{type(v)}` is not `int`")
        else:
            match += 1

        if match > 1:
            # more than 1 match
            raise ValueError("Multiple matches found when deserializing the JSON string into QueryStatusRequiredTime with oneOf schemas: int, none_type. Details: " + ", ".join(error_messages))
        elif match == 0:
            # no match
            raise ValueError("No match found when deserializing the JSON string into QueryStatusRequiredTime with oneOf schemas: int, none_type. Details: " + ", ".join(error_messages))
        else:
            return v

    @classmethod
    def from_dict(cls, obj: dict) -> QueryStatusRequiredTime:
        return cls.from_json(json.dumps(obj))

    @classmethod
    def from_json(cls, json_str: str) -> QueryStatusRequiredTime:
        """Returns the string representation of the model"""
        instance = cls()
        error_messages = []
        match = 0

        if match > 1:
            # more than 1 match
            raise ValueError("Multiple matches found when deserializing the JSON string into QueryStatusRequiredTime with oneOf schemas: int, none_type. Details: " + ", ".join(error_messages))
        elif match == 0:
            # no match
            raise ValueError("No match found when deserializing the JSON string into QueryStatusRequiredTime with oneOf schemas: int, none_type. Details: " + ", ".join(error_messages))
        else:
            return instance

    def to_json(self) -> str:
        """Returns the JSON representation of the actual instance"""
        if self.actual_instance is not None:
            return self.actual_instance.to_json()
        else:
            return "null"

    def to_dict(self) -> dict:
        """Returns the dict representation of the actual instance"""
        if self.actual_instance is not None:
            return self.actual_instance.to_dict()
        else:
            return dict()

    def to_str(self) -> str:
        """Returns the string representation of the actual instance"""
        return pprint.pformat(self.dict())

[awildturtok]

Sorry, i found these typos while testing

[wing328]

Sorry, i found these typos while testing

Thank you. I will fix those.

[LeulBM]

I appreciate how the new structure of the generated client helps with readability. Its easy to at a glance understand where code that controls a given path lives

[wing328]

The following component produces a broken from_json for requiredTime.

@awildturtok thanks for reporting the issue. I'll try to address it in another PR after merging this one.

[Linus-Boehm]

@wing328 Great work! Looks really good 👍 Are there already plans to integrate this beta version into one of the next releases or is there already a docker image published with these changes?

Edit: nvm, just found out that there is a docker build and pushed from master 👍

[AndreyDodonov-EH]

Pretty great work!

A question for understanding:
OAuth 2.0 flows still have to be implemented by the user, i.e.
obtaining and refreshing access_token handling outside of the generated code and just setting it in configuration object?

[wing328]

Right, those logics are not yet added to the auto-generated python sdk at the moment.

We definitely welcome PRs to add those enhancement to delivery a better experience using OAuth 2.0 with the auto-generated Python SDK.

[TimoGuenther]

Thank you! Using this instead of the default Python generator fixes a known bug with nullable references (#5180):

One issue is that at least one language generator (python-experimental) is ignoring the "nullable" attribute. I.e. in the case of Python, when the server returns a null value for "billTo", the client raises an exception because it did not expect a null value.

[spacether]

@TimoGuenther saying that the python/python-experimental generator has a bug on its nullable implementation is incorrect.

Nullable is working in the python client generator in v6.2.0 and onward (previously python-experimental). Verified by these tests:

• https://github.com/OpenAPITools/openapi-generator/blob/master/samples/openapi3/client/petstore/python/tests_manual/test_nullable_string.py
• https://github.com/OpenAPITools/openapi-generator/blob/master/samples/openapi3/client/petstore/python/tests_manual/test_fruit.py#L114

Nullable is not allowed as a keyword adjacent to $ref per the openapi 3.0.3 spec. In 3.1.0 it is allowed adjacent but our tooling cannot yet handle 3.1.0 specs. Per the 3.0.3 spec: This (ref) object cannot be extended with additional properties and any properties added SHALL be ignored.

[TimoGuenther]

@spacether, you are right that the python generator no longer has the issue with nullable described in the opening post of the issue I linked. I was actually having a slightly different issue with the same symptoms when using python. I was under the impression that the following construct correctly models a child property that may be null:

child:
  allOf:
  - $ref: '#/components/schemas/Child'
  nullable: true

After all:

• This is how the widespread DRF Spectacular generates a nullable object reference.
• It looks intuitive.
• It causes python-nextgen to create a child attribute that may be None.

However, according to the convoluted definition of nullable in the OpenAPI specification version 3.0.3, where nullable only does anything if it is true and adjacent to type, this nullable statement is actually a no-op. In other words, python is indeed working to specification, whereas python-nextgen is not.

[wing328]

query-status:
    type: object
    properties:
      requiredTime:
        oneOf:
          - type: integer
          - type: 'null'

@awildturtok we've added a rule in OpenAPI Normalizer to handle that: #14777

Please give it a try with the latest master and let me know if you're still facing the same issue.

Doc: https://github.com/OpenAPITools/openapi-generator/blob/master/docs/customization.md#openapi-normalizer

[Linus-Boehm]

I'm atm just using the models in conjunction with fastapi.
I encountered an issue with polymorphic schemas. We have 3 different schemas and have a discriminator on the type. Most of the fields are optional, which means the from_json try except logic will have multiple matches, which will lead to an error.

        try:
            instance.actual_instance = BooleanAnswer.from_json(json_str)
            match += 1
        except ValidationError as e:
            error_messages.append(str(e))
        # deserialize data into TextAnswer
        try:
            instance.actual_instance = TextAnswer.from_json(json_str)
            match += 1
        except ValidationError as e:
            error_messages.append(str(e))

I think in the sub-models, in this case eg. BooleanAnswer we need a check if the type matches and if not throw an exception or something

Happy to provide a reproduction and/or open a separate issue

[wing328]

@Linus-Boehm Did you try the latest master or release v6.4.0? We've made some improvements but not sure if it will address issue in your use case.

Can you also try enabling the option useOneOfDiscriminatorLookup to lookup based on type in your case?

[philippslang]

Tried this on one of our APIs and like it. One minor thing after first glance - can we remove the from __future__ import print_function in the readmes since we're >=3.7 anyways?

[wing328]

@philippslang thanks for the feedback. I've filed #15071 to remove it.

[philippslang]

Thanks @wing328 - think there's still a print function import in the generated readme. Also I think there s a bug for float values - it errors when the values in the json are ints (no decimal point). most of json renderers will omit a trailing .0

[wing328]

Also I think there s a bug for float values - it errors when the values in the json are ints (no decimal point). most of json renderers will omit a trailing .0

Are you using the latest master?

There's a new option to map the number.

	mapNumberTo
	    Map number to Union[StrictFloat, StrictInt], StrictStr or float. (Default: Union[StrictFloat, StrictInt])

The default should work for your use case.

[wing328]

about readme still referencing the import, it will be removed as part of #15380