FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-89083: add support for UUID version 8 (RFC 9562) by picnixz · Pull Request #123224 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-89083: add support for UUID version 8 (RFC 9562) - #123224

Merged
vstinner merged 14 commits into
python:mainfrom
picnixz:uuid-v8-89083
Nov 12, 2024
ConversationCommits (14)ChecksFiles changed
Merged

gh-89083: add support for UUID version 8 (RFC 9562)#123224
vstinner merged 14 commits into
python:mainfrom
picnixz:uuid-v8-89083

Conversation

picnixz commented Aug 22, 2024 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

Simplest implementation of UUIDv8. Extensible implementation where hash functions can be used may be a follow-up PR / feature.

Entropy considerations can be discussed directly on the PR since the issue is becoming larger and larger (but we could also consider opening a separate issue to discuss the implementation more in details).

📚 Documentation preview 📚: https://cpython-previews--123224.org.readthedocs.build/

bedevere-app Bot mentioned this pull request Aug 22, 2024
Closed
picnixz mentioned this pull request Aug 22, 2024
Merged
eendebakpt reviewed Aug 23, 2024
View reviewed changes
Comment thread Lib/uuid.py
vstinner reviewed Sep 18, 2024
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The change mostly LGTM, but there is missing documentation in uuid.rst ("TODO" ;-)).

Comment thread Lib/uuid.py Outdated
Comment thread Lib/uuid.py Outdated
Comment thread Lib/test/test_uuid.py
Comment thread Doc/library/uuid.rst Outdated

vstinner commented Sep 19, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

By the way, uuid8(a=None, b=None, c=None): I was surprised by the "generic" argument names (a, b, c), but RFC 9562 uses the same names (custom_a, custom_b, custom_c): https://www.rfc-editor.org/rfc/rfc9562.html#name-uuid-version-8

picnixz commented Sep 19, 2024

Copy link
Copy Markdown
Member Author

I had a little interrogation here. Should we use keyword-only parameters (if so, which names? with/without the custom_ prefix?)?

I also wondered whether to keep custom_a or a but if people want to use keywords (without the parameters being keyword-only) the first one does not give more information so the short name is probably preferred to reduce the number of keystrokes...

vstinner commented Sep 19, 2024

Copy link
Copy Markdown
Member

I'm fine with uuid8(a=None, b=None, c=None) API: short parameter names (not custom_ prefix) and allow passing arguments as positional arguments.

vstinner reviewed Sep 26, 2024
View reviewed changes
Comment thread Lib/uuid.py
b = random.getrandbits(12)
if c is None:
import random
c = random.getrandbits(62)

vstinner Sep 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Does it make sense / is it possible to reject values outside the expected value range? For example, reject negative numbers?

Maybe something like:

orig_a = a
a &= 0xffff_ffff_ffff
if a != orig_a: raise ValueError("...")

I don't know. Would it be consistent with other uuid functions?

picnixz Sep 26, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

UUIDv1 does not reject them so I wouldn't worry too much about it. v3 and v5 are not based on integral inputs.

vstinner Sep 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm fine with having the same behavior than uuid1() in this case, since it's documented.

vstinner approved these changes Sep 26, 2024
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM but I would prefer to have a second core dev to review the feature.

hugovk commented Nov 2, 2024

Copy link
Copy Markdown
Member

(What's New conflict resolved)

hugovk approved these changes Nov 11, 2024
View reviewed changes

hugovk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks!

Comment thread Doc/library/uuid.rst Outdated
Comment thread Doc/library/uuid.rst Outdated
Apply suggestions from code review
5e97cc3 
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
picnixz commented Nov 11, 2024
View reviewed changes
Comment thread Lib/test/test_uuid.py Outdated
picnixz commented Nov 11, 2024
View reviewed changes
Comment thread Doc/library/uuid.rst Outdated
Comment thread Doc/library/uuid.rst Outdated

picnixz commented Nov 11, 2024

Copy link
Copy Markdown
Member Author

Thanks Hugo for the review! I've changed the docstrings in tests to regular comments to avoid cluttering the output in verbose mode and used .. versionadded:: next.

vstinner merged commit 03924b5 into python:main Nov 12, 2024

vstinner commented Nov 12, 2024

Copy link
Copy Markdown
Member

Merged, thanks @picnixz.

picnixz deleted the uuid-v8-89083 branch November 14, 2024 13:12
picnixz added a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
pythongh-89083: add support for UUID version 8 (RFC 9562) (python#123224
52666f0 
)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
pythongh-89083: add support for UUID version 8 (RFC 9562) (python#123224
4dd8f23 
)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
JostMigenda mentioned this pull request Jul 8, 2025
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

eendebakpt eendebakpt left review comments
vstinner vstinner approved these changes
hugovk hugovk approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL