feat: add service codegen framework #497
Merged
Conversation
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
isaiahvita
reviewed
Feb 12, 2024
...hy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/GoIntegration.java
Show resolved
Hide resolved
codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoCodegenContext.java
Show resolved
Hide resolved
.../java/software/amazon/smithy/go/codegen/service/protocol/aws/AwsJson10ProtocolGenerator.java
Outdated
Show resolved
Hide resolved
...-codegen/src/main/java/software/amazon/smithy/go/codegen/service/ServiceDirectedCodegen.java
Outdated
Show resolved
Hide resolved
syall
suggested changes
Feb 12, 2024
There was a problem hiding this comment.
General comments:
- Rather than "Service", I think "Server" is a better name. This avoids any naming confusion for Service shapes, and is the recommended way to name Smithy plugins: https://smithy.io/2.0/guides/building-codegen/configuring-the-generator.html#how-to-name-codegen-plugins
- The Server plugin using DirectedCodegen is awesome. I think unifying the Go Integration interfaces would be better in the long term, especially if we want to write integrations that are applicable to both client / server codegen. Keeping
ArtifactTypefor nullable index check modes makes sense though. - nit: For all of the classes with changes in visibility, I am wondering if we should move them to an
internalpackage also, just to be sure no one uses it. Either way, we should be liberal with using the@SmithyInternalApiannotation.
codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/CodegenVisitor.java
Show resolved
Hide resolved
codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoCodegenContext.java
Outdated
Show resolved
Hide resolved
...go-codegen/src/main/java/software/amazon/smithy/go/codegen/service/GoServiceIntegration.java
Outdated
Show resolved
Hide resolved
...hy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/service/NoopServiceStruct.java
Outdated
Show resolved
Hide resolved
...-go-codegen/src/main/java/software/amazon/smithy/go/codegen/service/NotImplementedError.java
Outdated
Show resolved
Hide resolved
...-codegen/src/main/java/software/amazon/smithy/go/codegen/service/ServiceDirectedCodegen.java
Outdated
Show resolved
Hide resolved
...en/src/main/java/software/amazon/smithy/go/codegen/service/integration/DefaultProtocols.java
Outdated
Show resolved
Hide resolved
.../java/software/amazon/smithy/go/codegen/service/protocol/aws/AwsJson10ProtocolGenerator.java
Outdated
Show resolved
Hide resolved
codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoCodegenContext.java
Show resolved
Hide resolved
Comment on lines
137
to
142
| if (directive.shape().getId().getNamespace().equals(CodegenUtils.getSyntheticTypeNamespace())) { | ||
| return; | ||
| } | ||
| if (isUnit(directive.shape().getId())) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Is there a reason why we are skipping generating the synthetic I/O shapes and unit types? The I/O shapes should be generated.
There was a problem hiding this comment.
Unclear, this was taken from the client version.
There was a problem hiding this comment.
Marked as FUTURE to reconcile
- Remove document support code - Skip event streaming operations
lucix-aws
force-pushed
the
feat-svcgen
branch
from
677a2f1 to
70a8eaf
Compare
syall
approved these changes
Feb 27, 2024
Comment on lines
+52
to
+56
| @SmithyInternalApi | ||
| public enum ArtifactType { | ||
| CLIENT, | ||
| SERVER; | ||
| } |
There was a problem hiding this comment.
I don't think ArtifactType is doing anything meaningful right now, it can be removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Adds the infrastructure for service code generation.
Service codegen exists as a separate plugin which is powered by the newer recommended
DirectedCodegen.This PR adds partial support for the awsJson1.0 protocol. The document type and event stream operations are not yet implemented.
I had to change some internals to work w/
DirectedCodegenthat technically would be experienced by the downstream v2 SDK (e.g. refactoringGoWriterto extendSymbolWriterinstead ofAbstractCodeWriter. I checked and confirmed that the output of v2's codegen is completely unchanged against this.Special thanks to @syall and @isaiahvita without whom this would not have been possible!