search

aws-solutions / aws-data-lake-solution

A deployable reference implementation intended to address pain points around conceptualizing data lake architectures that automatically configures the core AWS services necessary to easily tag, search, share, and govern specific subsets of data across a business or with other external businesses.
https://aws.amazon.com/solutions/implementations/data-lake-solution/
Apache License 2.0
399 stars 160 forks source link

Update shortid characters #33

Closed luke459 closed 4 years ago

luke459 commented 5 years ago

Issue #24

Updated shortid character selection to replace - and _ with $ and @ for the generated package ID.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

jgc234 commented 4 years ago

@LukeMilbourne - does the $ and @ char break anything else (such as glue, athena, URLs, CLI, etc?).. You'd need to escape the $ in the CLI at least. I reckon the '_' char was OK, or did you find something else it broke? I only ran into problems with a leading '-'. If you just replace that one char, but with what? shortid.characters() wants 64 unique chars and wont allow a repeat. Another idea could be generate a normal shortid, but afterwards translate any instances of '-' to another safe char (ie, slightly less entropy), but lower risk or new chars breaking something downstream. Or only translate '-' if it was the first char.

beomseoklee commented 4 years ago

Thanks for your input, and we will consider this one in the next release.

georgebearden commented 4 years ago

Thank you - This change has been incorporated in the latest commit of this solution.