Skip to content

Support generating non-AWS client #1

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

Closed
wants to merge 5 commits into from
Closed

Support generating non-AWS client #1

wants to merge 5 commits into from

Conversation

gosar
Copy link

@gosar gosar commented Mar 23, 2021

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

@gosar
Copy link
Author

gosar commented Mar 23, 2021

One thing I wanted to improve - EndpointsInputConfig.endpoint is optional. So when a customer of the BootlegClient in instantiating an instance of the client and doesn't provide an endpoint in the constructor, it is not a compile error. Since it is marked optional in Endpoints.ts and that's used for AWS service clients where it should be optional, I couldn't change this easily. I'd like to discuss ideas on how to do this.

Copy link

@JordonPhillips JordonPhillips left a comment

Choose a reason for hiding this comment

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

Generally it would be nice to have a comment explaining why a thing was disabled for non-aws services. And feel free to throw exceptions for bad trait configurations

Comment on lines 125 to 127
private static boolean isAwsService(Shape serviceShape) {
return serviceShape.getTrait(ServiceTrait.class).isPresent();
}

Choose a reason for hiding this comment

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

Can't you just check for sigv4 instead? The ServiceIndex had a getAuthSchemes operation you can use to generically check for auth. If the problem ist aht you need the aws service trait to make these components work, then you should throw a codegen exception if the sigv4 trait is present but the aws service trait is not.

Copy link
Owner

Choose a reason for hiding this comment

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

I agree, I think a sigv4 check is more useful here.

Copy link
Author

Choose a reason for hiding this comment

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

See the latest diffs. I think the serviceShape.hasTrait(SigV4.class) check is sufficient, so I didn't use ServiceIndex. LMK if you think it is needed.

Comment on lines 125 to 127
private static boolean isAwsService(Shape serviceShape) {
return serviceShape.getTrait(ServiceTrait.class).isPresent();
}
Copy link
Owner

Choose a reason for hiding this comment

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

I agree, I think a sigv4 check is more useful here.

@adamthom-amzn adamthom-amzn force-pushed the ssdk branch 6 times, most recently from aa35d8a to 093ce24 Compare March 25, 2021 19:24
@adamthom-amzn
Copy link
Owner

One thing I wanted to improve - EndpointsInputConfig.endpoint is optional. So when a customer of the BootlegClient in instantiating an instance of the client and doesn't provide an endpoint in the constructor, it is not a compile error. Since it is marked optional in Endpoints.ts and that's used for AWS service clients where it should be optional, I couldn't change this easily. I'd like to discuss ideas on how to do this.

You can do something like this:

constructor(c: EndpointsInputConfig & { endpoint : string }) {
 ...
}

This will make endpoint required for your constructor, even though it's optional in EndpointsInputConfig.

gosar added 5 commits April 5, 2021 22:03
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 #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.
@gosar gosar requested a review from adamthom-amzn April 6, 2021 20:29
gosar added a commit to gosar/aws-sdk-js-v3 that referenced this pull request Apr 8, 2021
gosar added a commit to gosar/aws-sdk-js-v3 that referenced this pull request Apr 8, 2021
gosar added a commit to gosar/aws-sdk-js-v3 that referenced this pull request Apr 8, 2021
gosar added a commit to aws/aws-sdk-js-v3 that referenced this pull request Apr 15, 2021
* 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
JordonPhillips pushed a commit to JordonPhillips/aws-sdk-js-v3 that referenced this pull request Apr 19, 2021
* 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
JordonPhillips added a commit to aws/aws-sdk-js-v3 that referenced this pull request Apr 29, 2021
* 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]>
Linkgoron pushed a commit to Linkgoron/aws-sdk-js-v3 that referenced this pull request Jan 26, 2022
* chore: serialize rest json error code header (aws#2166)

* chore: short-circuit ssdk-incompatible integrations (aws#2167)

* chore: disable idempotency autofill import when not generating a client (aws#2181)

* Support generating non-AWS client (aws#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 (aws#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 (aws#2305)

Co-authored-by: Adam Thomas <[email protected]>
Co-authored-by: Jaykumar Gosar <[email protected]>
@gosar
Copy link
Author

gosar commented Feb 23, 2022

Moved to upstream repo - aws#2222

@gosar gosar closed this Feb 23, 2022
srchase pushed a commit to srchase/smithy-typescript that referenced this pull request Mar 21, 2023
* 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/aws-sdk-js-v3#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]>
srchase pushed a commit to smithy-lang/smithy-typescript that referenced this pull request Mar 23, 2023
* 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/aws-sdk-js-v3#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]>
srchase pushed a commit to smithy-lang/smithy-typescript that referenced this pull request Apr 17, 2023
* 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/aws-sdk-js-v3#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]>
srchase pushed a commit to srchase/smithy-typescript that referenced this pull request Apr 19, 2023
* 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/aws-sdk-js-v3#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]>
srchase pushed a commit to srchase/smithy-typescript that referenced this pull request Jun 1, 2023
* 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/aws-sdk-js-v3#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]>
srchase pushed a commit to srchase/smithy-typescript that referenced this pull request Jun 9, 2023
* 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/aws-sdk-js-v3#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]>
srchase pushed a commit to smithy-lang/smithy-typescript that referenced this pull request Jun 16, 2023
* 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/aws-sdk-js-v3#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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants