Switch to typescript and response streaming #162
Conversation
package.json
Outdated
| "test": "docker run --rm -v `pwd`:/var/task -it --entrypoint='node' public.ecr.aws/lambda/nodejs:14 /var/task/test-filesize/index.js", | ||
| "update-test-fixtures": "docker run --rm -v `pwd`:/var/task -it --entrypoint='node' public.ecr.aws/lambda/nodejs:14 /var/task/test-filesize/index.js --update-fixtures", | ||
| "build-zip": "rm lambda.zip; zip -r --exclude='node_modules/aws-sdk/*' --exclude='node_modules/animated-gif-detector/test/*' lambda.zip ./node_modules/ index.js proxy-file.js lambda-handler.js", | ||
| "build": "sam build -u", |
There was a problem hiding this comment.
Is sam actually added as a dependency? (Is it in aws-lambda?)
There was a problem hiding this comment.
No, it's part of https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/install-sam-cli.html. I think we need to document it in CONTRIBUTING.md though
src/lib.ts
Outdated
| } | ||
|
|
||
| // add invalid args | ||
| resolve({ data, info: { ...info, errors: errors.join(';') } }); |
There was a problem hiding this comment.
That's just what it did already and I didn't want to change the interface
| Handler: dist/lambda-handler.handler | ||
| Environment: | ||
| Variables: | ||
| S3_BUCKET: hmn-uploads |
There was a problem hiding this comment.
Can this use a dummy name if it's just a template? Don't want to risk it accidentally being used
There was a problem hiding this comment.
This is meant to be used to be able to do the integration test. There's no permissions to read / write the bucket it's just public
There was a problem hiding this comment.
I see. Can we use the test bucket in that case? (Per compliance, not using prod data for testing/development.)
There was a problem hiding this comment.
this is no different than having a test which does curl http://humanmade.com/robots.txt to check it's working, so I don't think that really falls under using prod data for testing / development. That's more to do with copying prod data to development, so it leaks data potentially to people who only have access to "test data".
|
Also, the coding style here is all over the place. Tachyon was already a bit not great on this, but would be a good time to clean that up |
Co-authored-by: Ryan McCue <[email protected]>
Co-authored-by: Ryan McCue <[email protected]>
Co-authored-by: Ryan McCue <[email protected]>
Co-authored-by: Ryan McCue <[email protected]>
|
Ok just need to decide on coding standards. At the moment the whole project uses Prettier with tabs. Do we want to use hm eslint-config? It hasn't been updated in 3 years so I wasn't sure whether we actually wanted to use that or not. |
|
@rmccue ok ready for another review. |
|
Fixes #3 |
| 'X-Amz-Date', | ||
| 'X-Amz-Security-Token', | ||
| ] as const; | ||
| const presignedParams: { [K in ( typeof presignedParamNames )[number]]?: string } = {}; // eslint-disable-line no-unused-vars |
There was a problem hiding this comment.
This would probably be a bit cleaner if you split the type out I think, since it's relatively complex?
There was a problem hiding this comment.
Hoisting it out of the function I don't think really does much to reduce the complexity
This is a major version, with the following:
Todo:
local-server