CB2-14227: Update cert-gen to remove snowball SQS pattern #191
Conversation
src/functions/certGen.ts
Outdated
| @@ -1,57 +1,26 @@ | |||
| import { DeleteObjectCommandOutput, PutObjectCommandOutput } from '@aws-sdk/client-s3'; | |||
| import { Callback, Context, Handler, SQSEvent, SQSRecord } from 'aws-lambda'; | |||
| import { Callback, Context, Handler, SQSBatchItemFailure, SQSBatchResponse, SQSEvent, SQSRecord } from 'aws-lambda'; | |||
There was a problem hiding this comment.
Can you remove this unused import please
| return this.certificateUploadService.removeCertificate(testResult); | ||
| } | ||
|
|
||
| private async create(testResult: ITestResult): Promise<PutObjectCommandOutput> { |
There was a problem hiding this comment.
It may be personal preference and appreciate it is the same amount of code, but I think as a team we we are trying to move away from .then / .catch in favour of using awaits, for readability.
const response: IGeneratedCertificateResponse = await this.certificateGenerationService.generateCertificate(testResult);
return this.certificateUploadService.uploadCertificate(response);
There was a problem hiding this comment.
Oh yeah absolutely in favour of removing promises +1
| @@ -68,7 +68,8 @@ | |||
| "node-yaml": "^4.0.1", | |||
| "reflect-metadata": "^0.1.13", | |||
| "ts-node-register": "^1.0.0", | |||
| "typedi": "^0.10.0" | |||
| "typedi": "^0.10.0", | |||
| "uuid": "^10.0.0" | |||
There was a problem hiding this comment.
Node ships a randomUUID method via the crypto module - are we able to use that and reduce an extra dependency and dev dep?
There was a problem hiding this comment.
Was using uuid just for validation so we didn't need to include manual regexs. Happy for alternatives though
There was a problem hiding this comment.
No problem, if we are using it to validate an already existing value, then this is fine 👍
Update cert-gen to remove snowball SQS pattern
Refactored the entry point handler to be more SOLID. The return type has been updated to include partial batch failures.
CB2-14227
Checklist
idrequired prepended with"test-"have been checked with automation testers and added