1527513 - Slugid library returns bytestrings, not strings

Status: RESOLVED FIXED

(Taskcluster :: Services, enhancement)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

https://github.com/taskcluster/slugid.py/

In Python 3, slugid.v4() and slugid.nice() return bytes, which makes them a little hard to include in JSON output or other useful places. Since the results are always pure ASCII, it might be better to just return a string.

John or Pete, thoughts on how to solve this? Or even if it should be solved?

Comment 1

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: patch

Since they're always within the ascii range, it's safe to store them as unicode or byte strings interchangably without memory overhead. I think it's reasonable to use the default string type for the version of Python being run.

Pete, if you agree and think this patch is good, I can land it.

Comment 2

[Pete Moore [:pmoore][:pete]]

I remember :aki did the work to make the library work under both python 2 and python 3, and at the time I recall he spent some time considering which types of string to use. I'm not much of a python expert, so I'll defer to him, as there may be reasons he chose bytes.

Comment 3

[Pete Moore [:pmoore][:pete]]

Reminder: if we update this in our repo, we may want to copy it to gecko too (it is vendored in firefox).

Comment 4

[Aki Sasaki (not active)]

If I specified bytes, it may be because the original py2-only code was dealing with binary encodings, and I made it explicit without changing the behavior. Given a choice of behaviors, I would prefer ascii-only strings to be a string rather than a bytestring.

Comment 5

[Aki Sasaki (not active)]

Comment on attachment 9043621 [details]
patch

Thanks!

Comment 6

[Aki Sasaki (not active)]

This should result in a major version bump. I see a number of slugid.nice().decode('utf-8') in scriptworker tests that we'll need to fix up. Should be easy enough.

Comment 7

[John Ford [:jhford] CET/CEST Berlin Time]

I've released Slug Id v2.0.0 on pypi.

Comment 8

[:gerard-majax]

(In reply to Aki Sasaki [:aki] from comment #6)

This should result in a major version bump. I see a number of slugid.nice().decode('utf-8') in scriptworker tests that we'll need to fix up. Should be easy enough.

Got tricked on my side as well, with code doing slugid.nice().decode() in taskcluster-github-decision and no pinned version.