-
Notifications
You must be signed in to change notification settings - Fork 4
chore: updating to v3 of s3 API @W-20084586@ #1536
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
base: main
Are you sure you want to change the base?
Conversation
| rules: { | ||
| 'no-shadow': 'off', | ||
| '@typescript-eslint/no-shadow': ['error'], | ||
| '@typescript-eslint/no-unused-vars': [ |
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.
Added this so that parameters that are prefixed with an underscore are exempt from the rule. This is a trick I picked up from @stephen-carter-at-sf .
| "@salesforce/ts-types": "^2.0.10", | ||
| "aws-sdk": "^2.1692.0", | ||
| "@smithy/node-http-handler": "^4.4.5", | ||
| "@smithy/types": "^4.8.1", |
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.
Technically, the most recent version of this module is 4.9.0, but using that version causes type-mismatches with aws-sdk-client-mock, so I set it to use the version that the mocking library uses.
| }; | ||
|
|
||
| const buildHttpOptions = (): { httpOptions: ClientConfiguration['httpOptions'] } | Record<string, never> => { | ||
| const buildRequestHandler = (): NodeHttpHandler => { |
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.
The HttpOptions type seems to have been basically completely removed, so I went with what the documentation recommends and used a handler instead.
| httpsAgent: agent.https, | ||
| } as NodeHttpHandlerOptions), | ||
| }); | ||
| return s3.send(new PutObjectCommand({ Bucket: bucket, Key: key, Body: body })); |
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 appears to be the preferred way to call commands now, even though the old style does technically work, my experience was that I had to change it to the new style in order for the upload.test.ts mocking to work properly. If we want to stick to the old style, I can do more testing to see if maybe I was just being foolish and it works properly after all.
| import { putObject } from '../../src/codeSigning/upload.js'; | ||
|
|
||
| describe('Upload', () => { | ||
| const $$ = new TestContext(); |
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 didn't dig deep enough to determine whether this is still necessary, so I just opted to keep it. I can look into whether it's safe to remove if you guys want.
3ab57ae to
a695368
Compare
9ba50f7 to
8c4ac84
Compare
What does this PR do?
Updates the S3 API from v2 to v3.
What issues does this PR fix or reference?
@W-20084586@