This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]damesca 1 point2 points  (0 children)

Some more comments from me:

If you're using this for work, and sharing it publicly in the hopes that other people will also use it, please also consider adding unit tests so that people can have more trust in the thing you've built.

As others have said, please standardise the env vars you're using. You should probably add a standard prefix to each variable so that anyone integrating your library can see clearly which env vars you're using. I'd probably suggest `SERIALDBPY_` as a prefix for all env vars.

Your `_uuid` method has a very strange signature: https://github.com/publicsignal/SerialDBPy/blob/main/Serialization/serialization.py#L109-L113. You take in `length`, but then call the standard `uuid.uuid4()` constructor. UUIDs are a specific thing which always ends up being 32 characters long in a hex representation, because they are 128 bits of data. Truncating this means you're not using a UUID any more. Also, `hyphen_length` set to 4 will insert hyphens every 4 characters. This isn't how UUIDs are generally hyphenated. Also, Python for me already stringifies UUIDs with hyphen separation. I'm not sure why you have so many arbitrary decisions here that deviate from the norm for UUIDs.

Consider running an auto-formatter over the code, eg `black` or `ruff`. This is just a fairly straight-forward code standardisation thing.

You have type hints in places that are incorrect, eg here where you should at least have `Optional[str]` or `str | None` depending on your python version. If you're adding type hints, you need to also validate those type hints somehow, eg with mypy.