-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@Viicos, currently there are some linter failures— you'll want to do |
Viicos
force-pushed
the
model-rebuild
branch
from
December 11, 2024 10:19
9f75014
to
dea30a0
Compare
eli-bl
previously requested changes
Dec 12, 2024
openapi_python_client/schema/openapi_schema_pydantic/path_item.py
Outdated
Show resolved
Hide resolved
5 tasks
dbanty
approved these changes
Dec 24, 2024
There was a problem hiding this 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!
Merged
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
toTrue
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 hasdefer_build
set; in this case we also want to defer build. Another case is when making use of theCallback
type alias; it isn't directly visible but it uses an unresolvable forward reference).Ultimately, in the module's
__init__.py
, we callmodel_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 onmain
(with Pydantic 2.9.2), some models such asComponents
are not built. While this can still work in some cases, it is advised not to do so (whenComponents
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:Or let
model_rebuild
use the namespace where it was called (in our case, all the imports are available, so it works).