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

deps(sdk): barebones general public client builder. #5940

Closed
wants to merge 25 commits into from

Conversation

Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Nov 6, 2024

Problem

Start the transition to sdkv3. This PR is just a start, and is intended to serve as an opportunity to look at the design before building on it more. This is still missing a lot of features (see future work).

Excessive line changes are due to package-lock.json changes from install sdk-v3 clients.

Solution

  • Do not overwrite existing builder to allow for incremental transition to V3.
  • A barebones working model, with the telemetry middleware implemented.
  • A proof of concept transition with EC2, unreleased nature makes it low-risk.
  • Expansion of testing for client construction.

Future Work


License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Hweinstock Hweinstock marked this pull request as ready for review November 6, 2024 23:31
@Hweinstock Hweinstock requested a review from a team as a code owner November 6, 2024 23:31
@Hweinstock Hweinstock changed the title deps(sdk): start general public client builder. deps(sdk): barebones general public client builder. Nov 7, 2024
packages/core/src/shared/awsClientBuilderV3.ts Outdated Show resolved Hide resolved
Output: string
InstanceId: string
}

export class Ec2Client {
export class Ec2Wrapper {
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity what was the idea between calling it Ec2Wrapper? Is it because it's a toolkit wrapper around the sdk v3 builder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it seems like a more fitting name, and also because the EC2 v3 exports its client as EC2Client (instead of EC2 as in v2) so want to avoid confusion between the names. Do you think that renaming the 'clients' to 'wrappers' as we migrate them makes sense?

@Hweinstock Hweinstock changed the base branch from master to feature/sdkv3 November 20, 2024 18:48
@Hweinstock Hweinstock requested a review from a team as a code owner November 22, 2024 20:17
@Hweinstock Hweinstock requested a review from a team as a code owner November 22, 2024 20:17
@Hweinstock Hweinstock marked this pull request as draft November 22, 2024 20:23
@Hweinstock Hweinstock changed the base branch from feature/sdkv3 to master November 22, 2024 20:24
@Hweinstock Hweinstock changed the base branch from master to feature/sdkv3 November 22, 2024 20:25
@Hweinstock
Copy link
Contributor Author

/retryBuilds

@Hweinstock
Copy link
Contributor Author

This PR is too large, and I am unable to resolve build problems. Moving to #6097

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.

2 participants