Add experimental Python codegen CLI scaffold#744
Conversation
Mark generation command examples as schematic and clarify option validation behavior. Exercise python -m smithy_python in a subprocess.
| @@ -0,0 +1 @@ | |||
| # Changelog | |||
There was a problem hiding this comment.
Note for reviewers: Our release infra will automatically handle version bumping and changelog management.
There was a problem hiding this comment.
I haven't talked much about this publicly since I'm still working on implementations and a blog post, but check a look at shape closures. This is a way to drive generation that doesn't root everything in a service.
What it means for you is relatively little, other than that you should tolerate generating without a set service. Current type codegen generates a synthetic one, but you shouldn't operate that way.
Computing a closure is beyond your scope as a non-java plugin. I would suggest rather that you generate what's given to you and put the onus on customers to use smithy-build transforms to narrow it down to what they want. Maybe later you could implement everything needed to compute it and relax that restriction.
There was a problem hiding this comment.
Nice, thanks for making us aware of this.
I agree with you. I think not requiring --service <service_id> is also an improved user experience. It's pretty easy to detect on our end. For the AWS services, we've automated our infra to do smithy select --selector service to find it for us. I think we can do that at the Python layer here. The only issue I see here is if someone defines two services (not sure if smithy allows it). But in that case, we'd ask them to specify.
|
|
||
| ## Commands | ||
|
|
||
| Generation is organized by artifact type: |
There was a problem hiding this comment.
But what if you could mix artifacts. You don't now - but you could. Types+client or types+server is an obvious combo that's easy to do without much difficulty.
smithy-python generate --modes client,typesYou could default that to just be client since most will want that.
There was a problem hiding this comment.
I'm a bit hesitant to combine multiple modes into one command since that complicates mode-specific options. I'm wondering if we instead could do something like smithy-python generate client ... --with-types or similar.
But honestly, I'm not sure what you mean by client+types compared to what we have now. Can you elaborate a bit?
| @dataclass(frozen=True, slots=True) | ||
| class _Invocation: | ||
| artifact: str | ||
| model_source: bytes |
There was a problem hiding this comment.
A readable would be better here I think. You don't want to have to buffer all of that only to pass it into json.reads or whatever else.
There was a problem hiding this comment.
I don't think a readable would provide much more value here. json.load just does fp.read() internally, and we need the whole AST in memory anyway to resolve shape references. Using an open readable also means owning when to close it. We can probably do that cleanly with a context manager, but seems unnecessary. Additionally, the file and stdin cases close differently so we'd also have to keep track of that.
Lmk if I'm missing something and happy to re-evaluate. Since this is all under the hood, we can also always add if later if we find a good reason to.
| } | ||
| ``` | ||
|
|
||
| Artifact-specific options will be appended to `command` after they are defined. |
There was a problem hiding this comment.
This is more for my own understanding: the existing Java generator receives service, module, and moduleVersion as plugin settings for each generated client. So in the future are we going to add options like the following?
{
"run::python-client": {
"command": [
"smithy-python",
"generate",
"client",
"--service",
"com.amazonaws.bedrockruntime#AmazonBedrockFrontendService",
"--module",
"aws_sdk_bedrock_runtime",
"--module-version",
"0.8.0"
]
}
}There was a problem hiding this comment.
Yup. I need to look into Jordon's comment to see if --service will still be needed. I'll also consider using --package-name which we derive the module name from w/ an optional override (.e.g a aws-sdk-s3 package name generates a aws_sdk_s3 module name). I also think --module-version should become --package-version.
We may also grow this overtime to add new options like --http-client aiohttp | httpx | awscrt
There was a problem hiding this comment.
Note that you can also put these into environment variables. The run plugin supports putting stuff there. So you could also accept them from the environment if you want.
jonathan343
left a comment
There was a problem hiding this comment.
Thanks @JordonPhillips!
I addressed a few of your comments. Others might need clarification or further discussion.
|
|
||
| ## Commands | ||
|
|
||
| Generation is organized by artifact type: |
There was a problem hiding this comment.
I'm a bit hesitant to combine multiple modes into one command since that complicates mode-specific options. I'm wondering if we instead could do something like smithy-python generate client ... --with-types or similar.
But honestly, I'm not sure what you mean by client+types compared to what we have now. Can you elaborate a bit?
| @dataclass(frozen=True, slots=True) | ||
| class _Invocation: | ||
| artifact: str | ||
| model_source: bytes |
There was a problem hiding this comment.
I don't think a readable would provide much more value here. json.load just does fp.read() internally, and we need the whole AST in memory anyway to resolve shape references. Using an open readable also means owning when to close it. We can probably do that cleanly with a context manager, but seems unnecessary. Additionally, the file and stdin cases close differently so we'd also have to keep track of that.
Lmk if I'm missing something and happy to re-evaluate. Since this is all under the hood, we can also always add if later if we find a good reason to.
There was a problem hiding this comment.
Nice, thanks for making us aware of this.
I agree with you. I think not requiring --service <service_id> is also an improved user experience. It's pretty easy to detect on our end. For the AWS services, we've automated our infra to do smithy select --selector service to find it for us. I think we can do that at the Python layer here. The only issue I see here is if someone defines two services (not sure if smithy allows it). But in that case, we'd ask them to specify.
|
I'm so used to reviewing other people's PRs I accidentally submitted my responses as a review. Sorry if it looks weird. I see duplicate responses now. |
Overview
This PR lays the foundation for a Python-native Smithy code generator. It adds an experimental
smithy-pythonpackage with the command-line interface that will eventually generate Python clients and standalone types packages.The CLI currently validates its inputs and then exits with an explicit “generation is not implemented yet” error. The existing Java generator remains the primary one while the Python implementation is developed incrementally.
Background
The Python generator will consume Smithy JSON AST models and produce packages compatible with the existing Smithy Python runtime. It is distributed separately from those runtime packages and is only needed during code generation.
Smithy’s
runplugin invokes the generator as an external process, allowing it to integrate with standard Smithy builds without being loaded into the Smithy CLI or implemented in Java. Read Smithy run plugin docs for more info.Changes Summary
smithy-pythonpackage.generate clientandgenerate typescommands.--modeland--output.runplugin invocation through standard input and the provided plugin environment.CLI smoke tests
Both generation commands accept a model and reach the expected placeholder behavior:
Both generation commands exit with status 1, as expected for the current implementation.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.