Skip to content

fix: import cast for required const properties, since it's used in the template #1153

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

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

benweint
Copy link
Contributor

@benweint benweint commented Oct 28, 2024

The template for const properties uses cast unconditionally, but the import for cast in ConstProperty.get_imports was conditioned on whether the property was required or not, leading to broken generated code for required const properties.

I'd be happy to add a test for this if desired! A cursory glance suggests that maybe the end_to_end tests would be the right place?

Resolves #1150

@dbanty
Copy link
Collaborator

dbanty commented Oct 28, 2024

Yeah, if you wouldn't mind adding a test so we can prevent this from regressing in the future, that'd be great!

@benweint benweint force-pushed the import-cast-when-needed branch from 0a0671b to 4ce002e Compare October 28, 2024 23:20
@benweint
Copy link
Contributor Author

Sure thing, added a test case to both of these files and regenerated baselines:

  • end_to_end_tests/baseline_openapi_3.0.json
  • end_to_end_tests/baseline_openapi_3.1.yaml

(If that's not the right place for these tests, let me know what you think would be a better home!)

@dbanty dbanty added this pull request to the merge queue Oct 28, 2024
Merged via the queue into openapi-generators:main with commit 40d63f9 Oct 28, 2024
22 checks passed
@knope-bot knope-bot bot mentioned this pull request Oct 28, 2024
github-merge-queue bot pushed a commit that referenced this pull request Nov 23, 2024
> [!IMPORTANT]
> Merging this pull request will create this release

## Fixes

- allow required fields list to be specified as empty (#651) (#1149)
- import cast for required const properties, since it's used in the
template (#1153)

Co-authored-by: knope-bot[bot] <152252888+knope-bot[bot]@users.noreply.github.com>
micha91 pushed a commit to micha91/openapi-python-client that referenced this pull request May 13, 2025
…e template (openapi-generators#1153)

The
[template](https://github.com/openapi-generators/openapi-python-client/blob/main/openapi_python_client/templates/property_templates/const_property.py.jinja#L2)
for `const` properties uses `cast` unconditionally, but the import for
`cast` in `ConstProperty.get_imports` was conditioned on whether the
property was required or not, leading to broken generated code for
required const properties.

I'd be happy to add a test for this if desired! A cursory glance
suggests that maybe the end_to_end tests would be the right place?

Resolves openapi-generators#1150
micha91 pushed a commit to micha91/openapi-python-client that referenced this pull request May 13, 2025
> [!IMPORTANT]
> Merging this pull request will create this release

## Fixes

- allow required fields list to be specified as empty (openapi-generators#651) (openapi-generators#1149)
- import cast for required const properties, since it's used in the
template (openapi-generators#1153)

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.

Generated code lacks import of typing.cast for required const properties
2 participants