Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the @mongodb-js/dl-center package from AWS SDK v2 to AWS SDK v3 for S3 operations, updating the DownloadCenter client implementation and adjusting dependencies accordingly.
Changes:
- Replaced
aws-sdkv2 S3 usage with@aws-sdk/client-s3(v3) commands/client. - Updated types around S3 uploads/downloads and made config uploads use an explicit UTF-8 Buffer.
- Skipped S3rver-backed integration tests due to incompatibilities introduced by the SDK switch.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/dl-center/src/download-center.ts | Migrates S3 implementation to AWS SDK v3 and updates related types/behavior. |
| packages/dl-center/src/download-center.spec.ts | Skips S3 integration tests impacted by the SDK migration. |
| packages/dl-center/package.json | Swaps dependency from aws-sdk v2 to @aws-sdk/client-s3 v3. |
| package-lock.json | Updates lockfile for AWS SDK v3 dependency tree and removes AWS SDK v2 packages. |
Comments suppressed due to low confidence (1)
packages/dl-center/src/download-center.spec.ts:100
- This integration test is skipped, leaving uploadConfig/downloadConfig behavior untested. After fixing the AWS SDK v3 S3 client setup for S3rver, this should be re-enabled (or replaced with a working equivalent).
it.skip('can upload and download a valid config file', async function () {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "node_modules/url": { | ||
| "version": "0.10.3", | ||
| "resolved": "https://registry.npmjs.org/url/-/url-0.10.3.tgz", | ||
| "integrity": "sha512-hzSUW2q06EqL1gKM/a+obYHLIO6ct2hwPuviqTTOcfFVc61UbfJ2Q32+uGL/HCPxKqrdGB5QUwIe7UqlDgwsOQ==", | ||
| "dependencies": { | ||
| "punycode": "1.3.2", | ||
| "querystring": "0.2.0" | ||
| } |
There was a problem hiding this comment.
"url" and "querystring" being removed here must relate to the extract-types change but not sure exactly how
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/dl-center/src/download-center.ts:191
uploadAsset()switched toPutObjectCommandInput['Body'], but the JSDoc above still documents the parameter type asContent(from the removed v2 AWS SDK types). Updating the JSDoc keeps the public API documentation consistent with the new signature.
async uploadAsset(
s3ObjectKey: string,
content: PutObjectCommandInput['Body'],
options: UploadAssetOptions = {},
packages/dl-center/src/download-center.ts:151
- The JSDoc still references the removed
Contenttype in the return annotation, which no longer matches the method signature after the AWS SDK v3 migration. Updating it will keep generated docs/type hints accurate for consumers.
* @param {string} s3ObjectKey -
* the s3 object key of the asset that has to be downloaded,
* ie. `my-project/asset.zip`.
* @return {(Promise<Content | undefined>)}
* @memberof DownloadCenter
Description
Upgrades dl-center to AWS v3 SDK
The consumer of this is hadron-build.
There are breaking changes here:
Open Questions
Checklist