-
Notifications
You must be signed in to change notification settings - Fork 614
Support generating non-AWS client #2222
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
Conversation
…rvices This is an initial version to get a working version of generated code that compiles without manual edits in smithy-typescript-ssdk-demo. I expect to make updates to this logic.
The minor ones from adamthom-amzn#1
AddAwsRuntimeConfigTest is checking for some behaviors from AddAwsAuthPlugin too, which was failing with missing aws.auth#sigv4 trait after my change. Added the trait for now to the test, but unit tests will need to be added/refactored for all these changes.
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.
We'll still need to support configuring the region even if it isn't being pulled from the aws config file if the service is using sigv4. Their region may be static or may not map to an AWS region, but something needs to be there for the request to be signed.
@@ -44,6 +45,7 @@ | |||
/** | |||
* Configure clients with AWS auth configurations and plugin. | |||
*/ | |||
// TODO: Think about AWS Auth supported for only some operations and not all, when not AWS service, with say @auth([]) |
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 the service has an auth trait you need to add the mechanics needed to support that regardless of what the default is (where @auth([])
is no auth by default). Since the auth
trait can only reference auth traits on the service, you don't have to worry about a single operation introducing its own auth. I'm pretty sure the current setup already handles operations with disabled auth.
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 put this TODO to think more deeply about different cases when I handle supporting SigV4 for non-AWS clients.
@@ -0,0 +1,37 @@ | |||
/* |
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.
There's already an AwsProtocolUtils
where this would fit
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 think AwsProtocolUtils
is for protocol specific utilities and this isn't about protocols, so I kept it separate.
...codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddAwsRuntimeConfig.java
Outdated
Show resolved
Hide resolved
...codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddAwsRuntimeConfig.java
Outdated
Show resolved
Hide resolved
...codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddAwsRuntimeConfig.java
Outdated
Show resolved
Hide resolved
...codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddAwsRuntimeConfig.java
Outdated
Show resolved
Hide resolved
@@ -33,12 +35,15 @@ | |||
/** | |||
* Add client plubins and configs to support injecting user agent. | |||
*/ | |||
// TODO: Looks to add this back for non-AWS service clients, by fixing the dependency on ClientSharedValues.serviceId |
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.
should be fairly easy to just write out the service shape name / id. This is what Go does
@JordonPhillips, thanks for the review. I addressed the code formatting comment. Re: sigv4, region, etc. yes I'm aware and those are next tasks for me. This version was supposed to be without sigV4 support. I also have a task to make UserAgent work again. Baby steps... |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
* feat(config-resolver): make region optional for non-AWS client * feat(codegen): skip integrations that are not relevant for non-AWS services This is an initial version to get a working version of generated code that compiles without manual edits in smithy-typescript-ssdk-demo. I expect to make updates to this logic. * chore(codegen): address code comments The minor ones from adamthom-amzn#1 * fix(codegen): use SigV4Trait check instead of ServiceTrait AddAwsRuntimeConfigTest is checking for some behaviors from AddAwsAuthPlugin too, which was failing with missing aws.auth#sigv4 trait after my change. Added the trait for now to the test, but unit tests will need to be added/refactored for all these changes. * chore(codegen): move isAwsService check to utils class * chore(codegen): code style formatting * chore(codegen): check SigV4 trait for including region
* chore: serialize rest json error code header (#2166) * chore: short-circuit ssdk-incompatible integrations (#2167) * chore: disable idempotency autofill import when not generating a client (#2181) * Support generating non-AWS client (#2222) * feat(config-resolver): make region optional for non-AWS client * feat(codegen): skip integrations that are not relevant for non-AWS services This is an initial version to get a working version of generated code that compiles without manual edits in smithy-typescript-ssdk-demo. I expect to make updates to this logic. * chore(codegen): address code comments The minor ones from adamthom-amzn#1 * fix(codegen): use SigV4Trait check instead of ServiceTrait AddAwsRuntimeConfigTest is checking for some behaviors from AddAwsAuthPlugin too, which was failing with missing aws.auth#sigv4 trait after my change. Added the trait for now to the test, but unit tests will need to be added/refactored for all these changes. * chore(codegen): move isAwsService check to utils class * chore(codegen): code style formatting * chore(codegen): check SigV4 trait for including region * chore(codegen,config-resolver): refactor how endpoint is resolved for non-AWS client (#2287) * chore(config-resolver): refactor EndpointsConfig for non AWS services This reverts an earlier change to EndpointsConfig.ts and instead provides the new functionality in separate CustomEndpointsConfig.ts. This will be used as a separate plugin for endpoint resolution in non AWS clients. * feat(codegen): Use separate CustomEndpointsConfig for non-AWS clients This depends on newly added CustomEndpointsConfig in @aws-sdk/config-resolver package. * test(config-resolver): add test for CustomEndpointsConfig (#2305) Co-authored-by: Adam Thomas <[email protected]> Co-authored-by: Jaykumar Gosar <[email protected]>
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
Description
These changes are to support generating a non-AWS typescript client. There are changes in the code generation logic to skip pieces that apply only to AWS services. Some typescript dependencies (aws-sdk/config-resolver) also needed updates.
This is initial prototype work for smithy-typescript-ssdk-demo.
Testing
Using these changes I successfully generated a working non-AWS typescript client for the bootleg-service in smithy-typescript-ssdk-demo.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.