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

Support Python codegen for the OpenAPI backend #21316

Merged
merged 21 commits into from
Dec 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Implement python codegen
```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",
    },
)
```
  • Loading branch information
grihabor committed Aug 11, 2024
commit 14a9762441c2401df9abef9f051854eeed283030
3 changes: 1 addition & 2 deletions src/python/pants/backend/openapi/codegen/java/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
from pants.backend.openapi.util_rules import generator_process, pom_parser
from pants.backend.openapi.util_rules.generator_process import (
OpenAPIGeneratorProcess,
OpenAPIGeneratorType,
)
from pants.backend.openapi.util_rules.pom_parser import AnalysePomRequest, PomReport
from pants.engine.fs import (
Expand Down Expand Up @@ -106,7 +105,7 @@ async def compile_openapi_into_java(
merged_digests = await Get(Digest, MergeDigests([request.input_digest, output_digest]))

process = OpenAPIGeneratorProcess(
generator_type=OpenAPIGeneratorType.JAVA,
generator_name='java',
argv=[
*(
("--additional-properties", f"apiPackage={request.api_package}")
Expand Down
35 changes: 18 additions & 17 deletions src/python/pants/backend/openapi/codegen/python/extra_fields.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
from pants.backend.openapi.target_types import OpenApiDocumentGeneratorTarget, OpenApiDocumentTarget
from pants.engine.target import BoolField, StringField
from pants.jvm.target_types import PrefixedJvmJdkField, PrefixedJvmResolveField
from pants.backend.python.target_types import PrefixedPythonResolveField
from pants.engine.target import BoolField, DictStringToStringField, StringField


class OpenApiPythonModelPackageField(StringField):
alias = "python_model_package"
help = "Root package for generated model code"
class OpenApiPythonGeneratorNameField(StringField):
alias = "python_generator_name"
required = False
help = "Python generator name"
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nothing is stopping an end user for typing python_generator_name="rust"

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. Moreover openapi-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.

simply misspelling the value

Yes, this is a good idea, we can use openapi-generator list for simple set validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added validation with OpenAPIGeneratorNames

Copy link
Contributor

@alonsodomin alonsodomin Dec 21, 2024

Choose a reason for hiding this comment

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

Moreover openapi-generator list doesn't tell us which language it generates

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.



class OpenApiPythonApiPackageField(StringField):
alias = "python_api_package"
help = "Root package for generated API code"
class OpenApiPythonAdditionalPropertiesField(DictStringToStringField):
alias = "python_additional_properties"
help = "Additional properties for python generator"


class OpenApiPythonSkipField(BoolField):
Expand All @@ -22,14 +23,14 @@ class OpenApiPythonSkipField(BoolField):
def rules():
return [
OpenApiDocumentTarget.register_plugin_field(OpenApiPythonSkipField),
OpenApiDocumentTarget.register_plugin_field(OpenApiPythonModelPackageField),
OpenApiDocumentTarget.register_plugin_field(OpenApiPythonApiPackageField),
OpenApiDocumentTarget.register_plugin_field(OpenApiPythonGeneratorNameField),
OpenApiDocumentTarget.register_plugin_field(OpenApiPythonAdditionalPropertiesField),
OpenApiDocumentGeneratorTarget.register_plugin_field(OpenApiPythonSkipField),
OpenApiDocumentGeneratorTarget.register_plugin_field(OpenApiPythonModelPackageField),
OpenApiDocumentGeneratorTarget.register_plugin_field(OpenApiPythonApiPackageField),
# Default Pants JVM fields
# OpenApiDocumentTarget.register_plugin_field(PrefixedJvmJdkField),
# OpenApiDocumentTarget.register_plugin_field(PrefixedJvmResolveField),
# OpenApiDocumentGeneratorTarget.register_plugin_field(PrefixedJvmJdkField),
# OpenApiDocumentGeneratorTarget.register_plugin_field(PrefixedJvmResolveField),
OpenApiDocumentGeneratorTarget.register_plugin_field(
OpenApiPythonAdditionalPropertiesField
),
OpenApiDocumentGeneratorTarget.register_plugin_field(OpenApiPythonGeneratorNameField),
# Default Pants python fields
OpenApiDocumentTarget.register_plugin_field(PrefixedPythonResolveField),
OpenApiDocumentGeneratorTarget.register_plugin_field(PrefixedPythonResolveField),
]
Loading