Seed DynamoDB Local#197
Conversation
Co-authored-by: Jon Gjengset <jon@thesquareplanet.com>
jonhoo
left a comment
jonhoo
left a comment
There was a problem hiding this comment.
Nice, yeah, this is way better, thank you!
Left one nit, but will merge and we can fix that in a follow-up.
| .expect("id is in projection") | ||
| .as_s() | ||
| .expect("id is of type string"); | ||
| ulid::Ulid::from_string(id).ok() |
There was a problem hiding this comment.
Shouldn't we just .expect here too? This is for testing after all, and all the IDs should be valid ULIDs.
There was a problem hiding this comment.
When testing, one can add some questions to dynamodb directly via the admin web UI. The ids of those manually added questions can be invalid ULIDs and so we are just skipping those questions, i.e. not going to vote for them in the vote-over-time task. That was the idea. We could add a comment on this, wdyt?
There was a problem hiding this comment.
I think we're probably better served by asserting here — the convenience of being able to use non-ULIDs is outweighed (I think) by the risk of us encoding the assuming of ULIDs elsewhere in the system that may break in more subtle ways
2 participants
Addresses thread