-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[Chore]Update aws-sdk dependencies to latest possible #13665
Conversation
@@ -164,7 +168,7 @@ export class AmazonAIConvertPredictionsProvider { | |||
const synthesizeSpeechCommand = new SynthesizeSpeechCommand({ | |||
OutputFormat: 'mp3', | |||
Text: input.textToSpeech?.source?.text, | |||
VoiceId: voiceId, | |||
VoiceId: voiceId as SynthesizeSpeechInput['VoiceId'], |
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.
All these casting is required since SDK updated the types to be stricter than before and this avoids breaking change and anything outside of this will throw an error from API itself
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.
LGTM.
Added a couple comments around data specific updates that where needed with the data-schema
version update that had some quality of life cx improvements that refined types.
@@ -195,79 +195,6 @@ describe('server generateClient', () => { | |||
); | |||
}); | |||
|
|||
test('can list with sort direction (ascending)', async () => { |
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.
Note
Confirm that this test is not relevant without using compound keys for sorting, which should be tested by data-schema directly instead of indirectly through this abstraction.
@@ -296,7 +223,9 @@ describe('server generateClient', () => { | |||
config: config, | |||
}); | |||
|
|||
const mockContextSpec = {}; | |||
const mockContextSpec = { |
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.
Note
Type changed from any
to specific type, which breaks this test, but the type would be required for a customers app to work.
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.
LGTM, thanks!
Performing yarn why and yarn audit post this fix,
|
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.
LGTM!
Description of changes
Update aws-sdk dependencies to latest (3.621.0) possible
Issue #, if available
Fixes: #13670
Description of how you validated changes
Checklist
yarn test
passesChecklist for repo maintainers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.