-
Notifications
You must be signed in to change notification settings - Fork 749
refactor(codecatalyst): migrate client to sdkv3 #6734
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
|
|
/runIntegrationTests |
| if (r.operation || r.params) { | ||
| log.error( | ||
| 'API request failed (time: %dms): %s\nparams: %O\nerror: %O\nheaders: %O', | ||
| timecost, | ||
| r.operation, | ||
| truncateProps(r.params, 20, ['nextToken']), |
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.
This code produced some nice, readable logs. Does the new client wrapper have that?
Is it feasible for the v3 client wrapper to have a call() function with similar features as this CC call() function? Maybe as a followup PR
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 have it logging this without the performance and headers information here:
aws-toolkit-vscode/packages/core/src/shared/awsClientBuilderV3.ts
Lines 202 to 207 in 4c190f4
| function logAndThrow(e: any, serviceId: string, errorMessageAppend: string): never { | |
| if (e instanceof Error) { | |
| recordErrorTelemetry(e, serviceId) | |
| getLogger().error('API Response %s: %O', errorMessageAppend, e) | |
| } | |
| throw e |
Problem
codecatalyst is still running on v2. Migrate to v3.
Solution
callwrapper to work with commands, rather than requests.tokeninto V3 clients.correctClockskewis enabled by default so it can be dropped: https://github.com/aws/aws-sdk-js-v3/blob/eae65cded5e703306346bdbd1b5de9d23050054a/UPGRADING.mdundefinedchecks in many places. This involves wrapping some type guards due the problem discussed here, where type guards can't propagate up function scope.AccessDeniedExceptionin CC, so this logic has to change.Verification
opencommands that deep link to console.Future Work
callfunction to the default middleware stack for v3 clients.name(or other) fields on service errors to make up for missingcodeproperty on errors in v3.feature/xbranches will not be squash-merged at release time.