chroma
chroma copied to clipboard
Use strings everywhere, not UUID + add more types
Description of changes
Adds a bunch of types which (i think) are correct ~ pls lmk if not. Also this should fix #126 by using Strings everywhere, even in Clickhouse where they have a uuid type. Think this makes sense since string uuids are more universally compatible with other possible future backends.
Summarize the changes made by this PR.
- Improvements & Bug fixes
- Adds a bunch of typing
- Removes use of UUID type and uses raw strings
- New functionality
- ...
Test plan
How are these changes tested? Current tests
Documentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository? Potentially
Hi August, appreciate all the changes! I think we want to keep our use of uuids as UUIDs internally to the databases and when they're passed around. Reason being:
- We want to be as storage sensitive as possible and the DBs here store uuids more efficiently when using their built in uuid type as opposed to strings (128bit int vs the hexadecimal string)
- The affordances of the UUID class are nice for users (.bytes / .hex) etc.
Not that hung up on 2nd point as you could always wrap it if you wanted.
Would you be able to change this to make it so that we use UUID over str?
@AugustKarlstedt we would love to get this merged! would it be ok with you if I hijack it and remove the UUID->string change for now?
hey @jeffchuber, totally cool w/ me!
@AugustKarlstedt this PR got pulled into the large backend refactor that @levand and @HammadB are working on. thanks for guiding us through it with this PR. Changes will be integrated with that PR when it lands.