Format s3 key#432
Conversation
eed86cf to
dd43cdf
Compare
e12a982 to
05b1949
Compare
dd43cdf to
28076c7
Compare
05b1949 to
cb7f48c
Compare
28076c7 to
755d067
Compare
755d067 to
800b2c9
Compare
| const hasExternalId = externalId && externalId.trim().length > 0; | ||
| const objectKey = hasExternalId | ||
| ? `${externalId}.json` | ||
| ? formatS3Key(externalId) |
There was a problem hiding this comment.
This looks good to me. Should the formatting be fingerpost-specific or apply to all items going via S3? If the latter, should we add this to the poller lambdas too?
There was a problem hiding this comment.
Yes, good thought. Do we store the original id anywhere with the poller lambdas?
I felt like doing some manipulation here was safe because we have the externalId in the database to cross reference.
Whereas with the poller lambda I don't know whether we might not want to risk any additional manipulation? Though, I also think it's nicer to store the records in this way.
@andrew-nowak do you have any thoughts?
| startDockerContainers | ||
| # run all three, and terminate together when the user presses `ctrl+c`: https://stackoverflow.com/a/52033580 | ||
| (trap 'kill 0' SIGINT; startIngestionLambda & startPlayApp & wait) | ||
| (trap 'kill 0' SIGINT; startQueueingLambda & startIngestionLambda & startPlayApp & wait) |
| const externalId = 'test-external-id'; | ||
| const expectedKey = 'test-external-id.json'; | ||
| expect(formatS3Key(externalId)).toBe(expectedKey); | ||
| }); | ||
| it('should replace spaces with a hyphen', () => { | ||
| const externalId = 'test external id'; | ||
| const expectedKey = 'test-external-id.json'; | ||
| expect(formatS3Key(externalId)).toBe(expectedKey); | ||
| }); | ||
| it('should replace forward slashes with a hyphen', () => { | ||
| const externalId = 'test/external/id'; | ||
| const expectedKey = 'test-external-id.json'; | ||
| expect(formatS3Key(externalId)).toBe(expectedKey); |
There was a problem hiding this comment.
Does this run the risk that some stories might overwrite each other in S3? if we had 3 files arrive with each of these external ids, then we'd be left with only the most recent, even though they've each got different ids. (Hopefully actually we still have all 3, but only as versions of object with the same key).
Do we have a concrete problem with the spaced or slashed keys? Maybe we run the id through a url encoder instead?
There was a problem hiding this comment.
The main thing with the spaced and slashed keys is we end up with a lot of documents with the key name as 0.json when the rest of the external id is stored as prefixes, which I was finding a bit confusing when I was trying to compare inputs.
Though yeah, I do get that doing our own manipulation could have some risks when we want to ensure the key is the identifier.
What would be the benefit of a URLEncoder instead?
There was a problem hiding this comment.
What do you both think? A problem worth addressing or prefer to leave as is?
3 participants
What does this change?
This formats the
externalIdto remove hyphens and spaces before we store it in s3How to test
How can we measure success?
Have we considered potential risks?
Images
Accessibility