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

Properly rebuild Pydantic models if necessary #1176

Merged
merged 5 commits into from
Dec 24, 2024

Conversation

Viicos
Copy link
Contributor

@Viicos Viicos commented Dec 5, 2024

This is the alternative approach I mentioned in #1171 (comment).

Instead of trying to rebuild the models in their respective modules (which requires weird patterns, such as unused imports or importing after the model is defined), we set defer_build to True for every model where we know a forward reference will fail to resolve (so that we don't try to build a model if we know it will fail).

I added comments each time to justify the use of defer_build, but unfortunately this isn't always straightforward (e.g. sometimes you makes use of a model as annotation which itself has defer_build set; in this case we also want to defer build. Another case is when making use of the Callback type alias; it isn't directly visible but it uses an unresolvable forward reference).

Ultimately, in the module's __init__.py, we call model_rebuild on all the necessary models. I know this isn't ideal as well, as you need to manually check for every exported model here if the build was successful.

This library is a clear example that inter-dependent types across different modules is challenging, and Pydantic does not make it easy. We are trying to think about ways to simplify the process.

Note that on top of fixing things for Pydantic 2.10, this also ensures every model is successfully built when the openapi_schema_pydantic module is imported. Currently on main (with Pydantic 2.9.2), some models such as Components are not built. While this can still work in some cases, it is advised not to do so (when Components is going to be instantiated, Pydantic will implicitly try to rebuild it if it wasn't already. However, we use the namespace where the instantiation call happened to rebuilt it, so depending on where you first instantiate the model, this can lead to a failed model rebuild and thus a runtime exception).


A note on model_rebuild: you can either provide an explicit namespace:

PathItem.model_rebuild(_types_namespace={Operation: "Operation", "Header": Header})

Or let model_rebuild use the namespace where it was called (in our case, all the imports are available, so it works).

@eli-bl
Copy link
Collaborator

eli-bl commented Dec 7, 2024

@Viicos, currently there are some linter failures— you'll want to do pdm lint && pdm format && pdm mypy.

@dbanty dbanty dismissed eli-bl’s stale review December 24, 2024 21:06

Changes have been addressed

Copy link
Collaborator

@dbanty dbanty left a comment

Choose a reason for hiding this comment

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

Thanks so much for submitting the fix!

@dbanty dbanty added this pull request to the merge queue Dec 24, 2024
Merged via the queue into openapi-generators:main with commit 800a715 Dec 24, 2024
19 checks passed
@knope-bot knope-bot bot mentioned this pull request Dec 24, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 24, 2024
> [!IMPORTANT]
> Merging this pull request will create this release

## Breaking Changes

### Delete fewer files with `--overwrite`

`--overwrite` will no longer delete the entire output directory before
regenerating. Instead, it will only delete
specific, known directories within that directory. Right now, that is
only the generated `models` and `api` directories.

Other generated files, like `README.md`, will be overwritten. Extra
files and directories outside of those listed above
will be left untouched, so you can any extra modules or files around
while still updating `pyproject.toml` automatically.

Closes #1105.

## Features

- Support httpx 0.28 (#1172)

### Add `generate_all_tags` config option

You can now, optionally, generate **duplicate** endpoint
functions/modules using _every_ tag for an endpoint,
not just the first one, by setting `generate_all_tags: true` in your
configuration file.

## Fixes

- Support Typer 0.14 and 0.15 (#1173)

### Fix minimum `attrs` version

The minimum `attrs` dependency version was incorrectly set to 21.3.0.
This has been corrected to 22.2.0, the minimum
supported version since `openapi-python-client` 0.19.1.

Closes #1084, thanks @astralblue!

### Fix compatibility with Pydantic 2.10+

#1176 by @Viicos

Set `defer_build` to models that we know will fail to build, and call
`model_rebuild`
in the `__init__.py` file.

Co-authored-by: knope-bot[bot] <152252888+knope-bot[bot]@users.noreply.github.com>
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.

3 participants