Skip to content
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 bug of generating enum class in python-fastapi generator #14242

Closed
wants to merge 1 commit into from

Conversation

liulu1998
Copy link

Fix #10127
Previous python-fastapi generator generated enum classes with empty body. In this PR, I try to generate Python's standard enum class, mapping values to str.

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. @spacether

@@ -18,6 +25,21 @@ from pydantic import AnyUrl, BaseModel, EmailStr, Field, validator # noqa: F401

{{#models}}
{{#model}}
{{#isEnum}}
class {{classname}}(str, Enum):
Copy link
Contributor

@spacether spacether Dec 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the enum is for schema integers, numbers, or boolean, or includes null?
This forces them all to inherit str

Copy link

@Linus-Boehm Linus-Boehm Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't enums in OpenApi not always string based?
https://swagger.io/docs/specification/data-models/enums/

The only way I found to specify integer like enums would be something like:

# Source: https://stackoverflow.com/questions/66465888/how-to-define-enum-mapping-in-openapi

Severity:
  type: integer
  oneOf:
    - title: HIGH
      const: 2
      description: An urgent problem
    - title: MEDIUM
      const: 1
    - title: LOW
      const: 0
      description: Can wait forever

But I think that would need anyways a different handling, as the example is not even typed as enum.

Even this being maybe not perfect, it is already a big improvement to no generation at all IMO.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enums can have any type. If type is unset enum values can be any json schema type

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide an example from openapi?

Copy link
Contributor

@spacether spacether Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, here is an enum that contains a string, int, float, and boolean

EnumOfAnyType:
  enum:
  - "a"
  - 1
  - 3.14
  - true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what we're looking for?

ReportingType:
  type: integer
  enum:
    - 1
    - 7

@Linus-Boehm
Copy link

Great initiative! @liulu1998 are you working on the tests or do you need support?

@liulu1998
Copy link
Author

@Linus-Boehm I'm working on those tests.

@maxiptah
Copy link

I've tried to generate a python-fastapi server now and get empty enum classes like this:

class DialogStartTypes(BaseModel):
    """NOTE"""

So this is I guess will be handled by this issue. Are you also aware that in endpoints enum type is not present, so the generated code is invalid:

@router.post(
    "/v1/whatever",
)
async def my_method_post(
    language_id: int = Query(None, description=""),
    dialog_start_type:  = Query(None, description="")
)

Note the int after language_id and lack of DialogStartTypes after dialog_start_type.
Is this also covered with your fix or should I open a separate bug?

@wing328
Copy link
Member

wing328 commented Feb 16, 2023

please give python-nextgen client generator a try as it also leverages pydantic.

if the implementation looks good, we can plot it to python-fastapi

@maxiptah
Copy link

python-nextgen is not listed, all I have is:

python-legacy
python
python-fastapi
python-prior
python-flask
python-aiohttp
python-blueplanet

@wing328
Copy link
Member

wing328 commented Feb 17, 2023

are you using openapi-generator v6.3.0?

@maxiptah
Copy link

i'm using openapi-generator-cli as NPM package:
"@openapitools/openapi-generator-cli": "^2.5.2"

@wing328
Copy link
Member

wing328 commented Feb 17, 2023

pls run openapi-generator-cli version-manager set 6.3.0 to use v6.3.0

@maxiptah
Copy link

python-nextgen worked as I would expect, both problems are not there: enum type correctly passed to the controller, enum has members.
I would expect the same from python-fastapi, otherwise, it's unfortunately unusable..

@Beaueve
Copy link
Contributor

Beaueve commented Jun 22, 2023

Any updates on this?

@peyerroger
Copy link
Contributor

Would be great if we had this fix in.

@peyerroger
Copy link
Contributor

please give python-nextgen client generator a try as it also leverages pydantic.

if the implementation looks good, we can plot it to python-fastapi

What would it mean to 'plot' it to python-fastapi. Could I somehow help?

@wing328
Copy link
Member

wing328 commented Aug 14, 2023

Hi all,

I've filed #16321 to improve pydantic support in python-fastapi generator (based on pydantic implementation in python client generator).

Can you please test the branch python-fastapi-pydantic and let me know how that goes?

My preliminary test result looks good but I would like more python-fastapi users to test it as I'm no expert in python-fastapi.

Thank you.

@wing328
Copy link
Member

wing328 commented Aug 16, 2023

To test the branch, simply run the following:

git clone https://github.com/openapitools/openapi-generator -b python-fastapi-pydantic
cd openapi-generator
mvn clean package -DskipTests -Dmaven.javadoc.skip=true
java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g python-fastapi -i modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o /tmp/python-fastapi/

@jonasheschl
Copy link
Contributor

jonasheschl commented Oct 17, 2023

Would it be possible to merge this onto master as a quickfix untill #16321 is ready? The current state of #16321 doesn't work with our OpenAPI documents sadly. The generated code is invalid Python. With this PR merged onto master to get the changes from #14470 for inversion of control, the generated FastAPI code works as expected for our OpenAPI documents.

@peyerroger
Copy link
Contributor

Hi all.

I did some tests. The model classes look ok. The problem with the enums still persists in the router classes. List enums work, but single enums do not. It creates empty types:

openapi: 3.0.0
servers:
  - url: 'http://petstore.swagger.io/v2'
info:
  description: >-
    This is a sample server Petstore server. For this sample, you can use the api key
    `special-key` to test the authorization filters.
  version: 1.0.0
  title: OpenAPI Petstore
  license:
    name: Apache-2.0
    url: 'https://www.apache.org/licenses/LICENSE-2.0.html'
tags:
  - name: pet
    description: Everything about your Pets
  - name: store
    description: Access to Petstore orders
  - name: user
    description: Operations about user
paths:
  /fake/query_param_enum:
    get:
      tags:
        - fake
      summary: test query parameter enums
      description: ''
      operationId: fake_query_param_enum
      parameters:
        - name: stringEnum
          in: query
          description: Uses an enum query param.
          schema:
            type: string
            enum:
              - a
              - b
      responses:
        '400':
          description: Invalid username supplied
        '404':
          description: User not found

It creates something like that. (Note the type missing after 'status:'):

stringEnum:  = Query(None, description="Uses an enum query param"),

@jonasheschl
Copy link
Contributor

Hi all.

I did some tests. The model classes look ok. The problem with the enums still persists in the router classes. List enums work, but single enums do not. It creates empty types:

openapi: 3.0.0
servers:
  - url: 'http://petstore.swagger.io/v2'
info:
  description: >-
    This is a sample server Petstore server. For this sample, you can use the api key
    `special-key` to test the authorization filters.
  version: 1.0.0
  title: OpenAPI Petstore
  license:
    name: Apache-2.0
    url: 'https://www.apache.org/licenses/LICENSE-2.0.html'
tags:
  - name: pet
    description: Everything about your Pets
  - name: store
    description: Access to Petstore orders
  - name: user
    description: Operations about user
paths:
  /fake/query_param_enum:
    get:
      tags:
        - fake
      summary: test query parameter enums
      description: ''
      operationId: fake_query_param_enum
      parameters:
        - name: stringEnum
          in: query
          description: Uses an enum query param.
          schema:
            type: string
            enum:
              - a
              - b
      responses:
        '400':
          description: Invalid username supplied
        '404':
          description: User not found

It creates something like that. (Note the type missing after 'status:'):

stringEnum:  = Query(None, description="Uses an enum query param"),

You can try #16321. Some people have reported better results with that branch.

@jonasheschl
Copy link
Contributor

Any update on this regarding a merge?

@wing328
Copy link
Member

wing328 commented Dec 18, 2023

please help test #17369 instead which uses pydantic v2

@liulu1998 liulu1998 closed this Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] python-fastapi generator problem with enumeration.
8 participants