-
-
Notifications
You must be signed in to change notification settings - Fork 640
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
base: main
Are you sure you want to change the base?
Conversation
```python openapi_document( name="java", source="document.json", skip_python=True, ) openapi_document( name="python", source="document.json", skip_java=True, python_generator_name="python", python_additional_properties={ "packageName": "version_service_client", "projectName": "version-service-client", }, ) ```
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.
Overall looks pretty good, just some suggestions around preventing the end users to shoot themselves in the foot in case they input the wrong generator name. The final implementation doesn't need to look exactly like that, just throwing some ideas to the wall and see what sticks.
class OpenApiPythonGeneratorNameField(StringField): | ||
alias = "python_generator_name" | ||
required = False | ||
help = "Python generator name" |
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`:
class OpenApiGeneratorNameField(StringField):
required = False
default: ClassVar[str]
value: str
src/python/pants/backend/openapi/codegen/python/extra_fields.py
:
class PythonOpenApiGeneratorNameField(OpenApiGeneratorNameField):
alias = "python_generator_name"
default = "python"
valid_choices = ("python", "python-pydantic-v1")
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.
openapi_document( | ||
name="petstore", | ||
source="petstore_spec.yaml", | ||
python_generator_name="python", |
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.
since we are giving freedom to the end user to choose the generator, may be worth having an additional test in which the generator being used is python-pydantic-v1
class OpenAPIGeneratorType(Enum): | ||
JAVA = "java" | ||
|
||
|
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.
If following the previously suggested approach of having a validation rule for the generator name. This could be replaced by a mapping that gets cached:
@dataclass(frozen=True)
class OpenAPIGeneratorNameMapping:
mapping: FrozenDict[str, tuple[str, ...]]
def generators_for(self, lang: str) -> tuple[str, ...]:
valid_choices = self.mapping.get(lang)
if valid_choices is None:
raise ValueError(f"Unsupported language generator: {lang}")
return valid_choices
def all_languages(self) -> tuple[str, ...]:
# return a tuple with the supported languages
def all_generator(self) -> tuple[str, ...]:
# return a tuple with all the generator names.
async get_openapi_generator_name_mapping(openapi_generator: OpenAPIGenerator) -> OpenAPIGeneratorNameMapping:
# Run the JVM process for the generator passing arguments `list -s`
# Parse CSL result from stdout. Generator names are always prefixed by the language name, e.i.: `java`, `java-helicon-client`, `python`, etc.
# https://openapi-generator.tech/docs/generators
Then the OpenAPIGeneratorNameMapping
could be used as the basis for a validation of the generator_name
field at different stages.
We've just branched for 2.23, so merging this pull request now will come out in 2.24, please move the release notes updates to |
No description provided.