-
-
Notifications
You must be signed in to change notification settings - Fork 643
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
Support Python codegen for the OpenAPI backend #21316
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
e0dd5dd
Copy openapi java stuff
grihabor 14a9762
Implement python codegen
grihabor 4343919
Change package structure
grihabor f5a3caa
Implement package_mapper
grihabor 22d4f6a
One test works
grihabor 581a51c
Some tests works
grihabor dc0a03a
Tests pass
grihabor b9973a3
pants fix
grihabor 4d6eb01
Fix mypy
grihabor b854cc1
pants fix
grihabor 5899574
Add notes
grihabor 12730a0
Set 2024
grihabor 6006a84
Use itertools.product
grihabor 4e9904e
Update notes
grihabor ee0a9bc
Update year
grihabor cc480a7
Revert notes
grihabor 350d3fd
Merge branch 'main' into openapi-python
grihabor 47228fe
Add a test for python-fastapi generator
grihabor e25726d
Add test_openapi_generator_name_validation
grihabor 3e882ed
Add notes
grihabor 2ec8ea2
Merge branch 'main' into openapi-python
alonsodomin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
4 changes: 4 additions & 0 deletions
4
src/python/pants/backend/experimental/openapi/codegen/python/BUILD
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md). | ||
# Licensed under the Apache License, Version 2.0 (see LICENSE). | ||
|
||
python_sources() |
Empty file.
18 changes: 18 additions & 0 deletions
18
src/python/pants/backend/experimental/openapi/codegen/python/register.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md). | ||
# Licensed under the Apache License, Version 2.0 (see LICENSE). | ||
|
||
from __future__ import annotations | ||
|
||
from pants.backend.experimental.openapi.register import rules as openapi_rules | ||
from pants.backend.experimental.openapi.register import target_types as openapi_target_types | ||
from pants.backend.experimental.python.register import rules as python_rules | ||
from pants.backend.experimental.python.register import target_types as python_target_types | ||
from pants.backend.openapi.codegen.python.rules import rules as openapi_python_codegen_rules | ||
|
||
|
||
def target_types(): | ||
return [*python_target_types(), *openapi_target_types()] | ||
|
||
|
||
def rules(): | ||
return [*python_rules(), *openapi_rules(), *openapi_python_codegen_rules()] |
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md). | ||
# Licensed under the Apache License, Version 2.0 (see LICENSE). | ||
|
||
python_sources() | ||
|
||
python_tests(name="tests", dependencies=[":lockfiles"]) | ||
|
||
resources(name="lockfiles", sources=["*.test.lock"]) |
Empty file.
42 changes: 42 additions & 0 deletions
42
src/python/pants/backend/openapi/codegen/python/extra_fields.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md). | ||
# Licensed under the Apache License, Version 2.0 (see LICENSE). | ||
import itertools | ||
|
||
from pants.backend.openapi.target_types import OpenApiDocumentGeneratorTarget, OpenApiDocumentTarget | ||
from pants.backend.python.target_types import PrefixedPythonResolveField | ||
from pants.engine.target import BoolField, DictStringToStringField, StringField | ||
|
||
|
||
class OpenApiPythonGeneratorNameField(StringField): | ||
alias = "python_generator_name" | ||
required = False | ||
help = "Python generator name" | ||
|
||
|
||
class OpenApiPythonAdditionalPropertiesField(DictStringToStringField): | ||
alias = "python_additional_properties" | ||
help = "Additional properties for python generator" | ||
|
||
|
||
class OpenApiPythonSkipField(BoolField): | ||
alias = "skip_python" | ||
default = False | ||
help = "If true, skips generation of Python sources from this target" | ||
|
||
|
||
def rules(): | ||
return [ | ||
target.register_plugin_field(field) | ||
for target, field in itertools.product( | ||
( | ||
OpenApiDocumentTarget, | ||
OpenApiDocumentGeneratorTarget, | ||
), | ||
( | ||
OpenApiPythonSkipField, | ||
OpenApiPythonGeneratorNameField, | ||
OpenApiPythonAdditionalPropertiesField, | ||
PrefixedPythonResolveField, | ||
), | ||
) | ||
] |
Oops, something went wrong.
Oops, something went wrong.
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.
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.
I see the potential for having an abstract
OpenApiGeneratorNameField
that is common to all codegen implementations and then extended in each of them narrowing down the possible values while providing also a default.Something like:
´src/python/pants/backend/openapi/codegen/extra_fields.py`:
src/python/pants/backend/openapi/codegen/python/extra_fields.py
:Similar thing for the Java codegen as apparently there are new (beta) generators available.
The reason I'm suggesting this is because nothing is that like it's been implemented, nothing is stopping an end user for typing
python_generator_name="rust"
or simply misspelling the value, which would lead to an execution error that may be hard to decipher.This has the obvious consequence of having to update the list of valid choices for each codegen when new ones are available, which can be a PITA.
A way to prevent having to update our sources would be to instead have a validation rule that obtains the list of all supported generators from the tool, the command
openapi-generator list -s
should return such a list in a comma-separated list.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.
I don't think this is the problem we need to solve. Similarly, one could use
python_sources(sources=["*.rs"])
, we don't want to validate such a thing. Moreoveropenapi-generator list
doesn't tell us which language it generates, which makes sense because the generator can actually generate multiple different kinds of files, like .py, .txt, .yaml. Anyway, the validation will create more problems than it will solve.Yes, this is a good idea, we can use
openapi-generator list
for simple set validation.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.
Added validation with
OpenAPIGeneratorNames
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.
I rather take it more like the generator targets a language + tooling, it does generate support files that are not in the target language, this is because it aims at generating a self-contained buildable product. And this isn't 100% correct either as it also can generate docs and Apache httpd configuration files so I understand that hardcoding an assumption into their naming convention could be problematic. Anyway, at least validating that the chosen name is an accepted one before running the tool gives a better UX.