Add OpenAPI 3.1.0 support

[dbanty]

TODO:

• Get tests passing again
• Put back the special-case "Unset is None" code for query params (decided to remove special case instead)
• Get coverage back up
• Get some people to test with real 3.0 AND 3.1 specs to make sure it's all working as intended
• Update CONTRIBUTING.md with new instructions (now that there are multiple e2e tests)
• Make the templates output nicer code (probably) since my current edits are haphazard

[codecov]

Codecov Report

Attention: 33 lines in your changes are missing coverage. Please review.

Comparison is base (f0cf8d2) 100.00% compared to head (4749528) 98.58%.
Report is 3 commits behind head on main.

❗ Current head 4749528 differs from pull request most recent head cf7d94e. Consider uploading reports for the commit cf7d94e to get more accurate results

Files	Patch %	Lines
openapi_python_client/parser/properties/const.py	77.77%	14 Missing ⚠️
openapi_python_client/parser/properties/union.py	94.31%	5 Missing ⚠️
openapi_python_client/parser/properties/any.py	86.95%	3 Missing ⚠️
openapi_python_client/parser/properties/boolean.py	91.17%	3 Missing ⚠️
openapi_python_client/parser/properties/none.py	93.54%	2 Missing ⚠️
openapi_python_client/parser/properties/file.py	96.42%	1 Missing ⚠️
openapi_python_client/parser/properties/float.py	97.05%	1 Missing ⚠️
openapi_python_client/parser/properties/int.py	97.05%	1 Missing ⚠️
...i_python_client/parser/properties/list_property.py	97.77%	1 Missing ⚠️
...penapi_python_client/parser/properties/protocol.py	98.66%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@             Coverage Diff             @@
##              main     #856      +/-   ##
===========================================
- Coverage   100.00%   98.58%   -1.42%     
===========================================
  Files           49       61      +12     
  Lines         1940     2330     +390     
===========================================
+ Hits          1940     2297     +357     
- Misses           0       33      +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[frco9]

Hello,
I've tested on a swagger generated by the last version of FastAPI (0.104.0)
Generation fails on a custom header we have in each endpoints. If I remove it, the generated client is working fine.

The error is :

WARNING parsing GET /users/{userId} within user. Endpoint will not be generated.

Union[None, Unset, str] is not allowed in header

Parameter(name='XX-Act-As', param_in=<ParameterLocation.HEADER: 'header'>, description='This header allows a restricted list of admin user to impersonate a user.', required=False, deprecated=False, allowEmptyValue=False, style=None, explode=False, allowReserved=False, param_schema=Schema(title=None, multipleOf=None, maximum=None, exclusiveMaximum=None, minimum=None, exclusiveMinimum=None, maxLength=None, minLength=None, pattern=None, maxItems=None, minItems=None, uniqueItems=None, maxProperties=None, minProperties=None, required=None, enum=None, type=None, allOf=[], oneOf=[Schema(title=None, multipleOf=None, maximum=None, exclusiveMaximum=None, minimum=None, exclusiveMinimum=None, maxLength=None, minLength=None, pattern=None, maxItems=None, minItems=None, uniqueItems=None, maxProperties=None, minProperties=None, required=None, enum=None, type=<DataType.STRING: 'string'>, allOf=[], oneOf=[], anyOf=[], schema_not=None, items=None, properties=None, additionalProperties=None, description=None, schema_format=None, default=None, nullable=False, discriminator=None, readOnly=None, writeOnly=None, xml=None, externalDocs=None, example=None, deprecated=None), Schema(title=None, multipleOf=None, maximum=None, exclusiveMaximum=None, minimum=None, exclusiveMinimum=None, maxLength=None, minLength=None, pattern=None, maxItems=None, minItems=None, uniqueItems=None, maxProperties=None, minProperties=None, required=None, enum=None, type=<DataType.NULL: 'null'>, allOf=[], oneOf=[], anyOf=[], schema_not=None, items=None, properties=None, additionalProperties=None, description=None, schema_format=None, default=None, nullable=False, discriminator=None, readOnly=None, writeOnly=None, xml=None, externalDocs=None, example=None, deprecated=None)], anyOf=[], schema_not=None, items=None, properties=None, additionalProperties=None, description='This header allows a restricted list of admin user to impersonate a user.', schema_format=None, default=None, nullable=False, discriminator=None, readOnly=None, writeOnly=None, xml=None, externalDocs=None, example=None, deprecated=None), example=None, examples=None, content=None)

In fastAPI I declare the following Header in each endpoints (using a Depends) :

def get_current_user(
    x_act_as: Union[str, None] = Header(
        None,
        alias="XX-Act-As",
        description=(
            "This header allows a restricted list of admin user to impersonate "
            "a user."
        ),
    ),
    (...)
)    

Which output the following Openapi.yaml :

(...)
get:
      operationId: get_user
      parameters:
      - description: Id of the user to fetch
        in: path
        name: userId
        required: true
        schema:
          description: Id of the user to fetch
          format: uuid
          type: string
      - description: This header allows a restricted list of admin user to impersonate
          a user.
        in: header
        name: XX-Act-As
        required: false
        schema:
          description: This header allows a restricted list of admin user to impersonate
            a user.
          oneOf:
          - type: string
          - type: 'null'
(...)

[dbanty]

Thanks for reporting @frco9, this is exactly the kind of feedback I'm hoping to get before releasing this! I believe I fixed it in the latest commit

[frco9]

Indeed it's working better @dbanty. Everything else seems ok on our side. Will let you know if we have other bugs

[frco9]

Well we found another issue with const :

We had in our 3.0 openapi a field used as discriminator with type a Literal from an Enum:

class DatasetDB(Dataset):
    dataset_type_code: Literal[DatasetTypeCode.TABLE] = Field(
        DatasetTypeCode.TABLE, description="Dataset's type. Set to Table"
    )
(...)

The diff openapi is :

(...)
        datasetTypeCode:
          default: table
          description: Dataset's type. Set to Table
-         enum:
-         - table
-         type: string
+         const: table
(...)

Which is changed to a const in the 3.1 version

While the changes for the generated client are :

- dataset_type_code: Union[Unset, DatasetDBDatasetTypeCode] = DatasetDBDatasetTypeCode.TABLE
+ dataset_type_code: Union[Unset, Any] = UNSET

I think proper output for client should be :

dataset_type_code: Union[Unset, Literal["table"]] = "table"

Thanks !

[dbanty]

@frco9 I added the most basic form of const support I could muster 😁. Please try it out again when you get the chance and find any other FastAPI features I'm missing!

[vogre]

This branch seems to work well with all OpenAPI 3.1.0 schemas that we've thrown at it 💯

[naddeoa]

I tested the branch locally on our schema and it does look like it works, but I noticed that its used attrs instead of dataclass. This has the side effect that pyrightt/pylance doesn't work correctly.

Is this a conscious choice? This milestone might be a good time to switch over to dataclasses.

An alternative solution would be to make sure the name of the variable is the same name as the kwarg. Pyright kind of gets the idea if those two things line up, but the AuthenticatedClient renamed all of the required kwargs to have a leading underscore.

[dbanty]

@naddeoa I don't remember why attrs was originally picked. There has been a bit of discussion around this (for example #574), but it is pretty low on my priority list to change that. The amount of work it would take to validate the idea is probably better spent adding support for more OpenAPI features (like #801, which is probably easier to do with attrs than dataclass).

[naddeoa]

@dbanty Do you know why the variable names are prefixed with _ for the client classes? I know its a sort of python convention, but attrs apparently takes the stance that privacy doesn't exist in Python and they strip them out anyway. Plus, not all of the variables have the leading underscore (like token and prefix) but I don't see a pattern to explain the differences.

Pyright would actually work out with attrs (from my testing locally) if the names just didn't include the leading underscore on the classes that people interface with directly.

[dbanty]

Do you know why the variable names are prefixed with _ for the client classes?

The properties of the class are prefixed to indicate that they are private and you shouldn't access them directly (because there are methods for that which do extra stuff).

Attrs correctly names the parameters in the constructor without that prefix because parameter names are not private.

For the client classes in particular, it might be better to use traditional Python classes instead of attrs classes. But again, not the highest priority.

I've never had luck with pyright, I always end up back with mypy when I try other type checkers 😅

[naddeoa]

For the client classes in particular, it might be better to use traditional Python classes instead of attrs classes. But again, not the highest priority.

I've never had luck with pyright, I always end up back with mypy when I try other type checkers 😅

I bring up pyright because the thing that drove me to this project was the goal of generating a "modern" client. Pyright/pylance seem to have all of the momentum and, from experience, they really do a much better job than mypy at so many issues, from generics to inference without type stubs, its been a real joy. I highly recommend. With the current state of the attr var names though, the auto generated stubs would generate errors for anyone using the "strict" pyright setting, which isn't a small number of people.

Do you think naming them without underscores, and sacrificing the thin veil of privacy that isn't really enforced anyway in Python, is a good trade off to support the strict pyright users? I lean towards yes, in the aim of generating a modern client. I can see some people disagreeing of course, but having the strictest type checkers be ok with the generated code seems like a tangible goal with lots of benefits.

Of course, its still light years better than the default openapi generator for Python, which I would honestly rather write a client from scratch than actually use nowadays.

[dbanty]

@naddeoa I created #909 to discuss further and so if this is important to other folks they can upvote it 🙂