chore: Uprade aws-sdk to v3 #3989
Conversation
There was a problem hiding this comment.
This is one where I'm not sure what to do. There seem to be various suggestions in the docs e.g. new CreateBucketsCommand, replace S3 with S3Client but I suppose I'm not altogether clear on what the Bucket part of the code is doing, and there are red squiggly lines over everything!
|
|
||
| return { | ||
| body: file.Body as Buffer, | ||
| body: file.Body, |
There was a problem hiding this comment.
Removing this causes type issues in modules/file/controller because it expects the body to be a Buffer - Argument of type 'StreamingBlobPayloadOutputTypes | undefined' is not assignable to parameter of type 'Buffer | undefined'.
There was a problem hiding this comment.
There is a similar issue in exportZip.ts - https://github.com/theopensystemslab/planx-new/actions/runs/12050192003/job/33598553246?pr=3989
|
Removed vultr server and associated DNS entries |
eea0e83 to
76f98ff
Compare
| @@ -22,7 +22,6 @@ MICROSOFT_CLIENT_SECRET=👻 | |||
|
|
|||
| # AWS credentials for uploading user files from local and pull request environments to a staging S3 bucket | |||
| AWS_S3_REGION=eu-west-2 | |||
| AWS_S3_ACL=public-read | |||
There was a problem hiding this comment.
This env var is actually static and never change, I've simply removed it and hardcoded it here -
| mockPutObject = vi.fn(() => ({ | ||
| promise: () => Promise.resolve(), | ||
| })); |
There was a problem hiding this comment.
This is a common pattern - many of these functions are now async by default and we no longer need to chain .promise().
| "s3.eu-west-2.amazonaws.com", | ||
| ); | ||
| it(`does not use Minio config on ${env} environment`, async () => { | ||
| vi.stubEnv("NODE_ENV", env); |
There was a problem hiding this comment.
This is a really nice little helper vitest provides for testing.
| import { isLiveEnv } from "../../../helpers.js"; | ||
|
|
||
| export function s3Factory() { | ||
| return new S3({ | ||
| params: { Bucket: process.env.AWS_S3_BUCKET }, |
There was a problem hiding this comment.
Bucket is now passed into the PutObject and GetObject commands.
| Body: file?.buffer || JSON.stringify(file), | ||
| Body: file.buffer, | ||
| ContentDisposition: `inline;filename="${filename}"`, | ||
| ContentType: file?.mimetype || "application/json", |
There was a problem hiding this comment.
These || statements are unreachable and can be safely removed.
There was a problem hiding this comment.
Thanks both for tag-teaming this one, some nice wins here!
One minor comment re: a test, but also want to flag I'm not able to load flows on this pizza (sharedb issue I think? teams & flow lists load fine), and would like to manually double check our send-to-email zip before final approval !
Take a look at Pizza now, thanks for raising this. |
|
@jessicamcinchak Pizza up and running 🍕 |
There was a problem hiding this comment.
Thanks for fixing pizza - double-checked files in submission zips all working as expected ✅ https://api.3989.planx.pizza/admin/session/210c2ff2-8358-47aa-8081-384cd1cdfe96/zip
Happy for this to merge !
I'm not particularly familiar with configuring AWS so have found this quite tricky, despite the helpful codemod script recommended by AWS.
I've added some comments on the code below with some of the issues/questions I've had!
Picking up from the work @jamdelion has started here!
What does this PR do?