-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
bpo-22807: Expose platform UUID generation safety information. #138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
bbdef4c
ccefb3a
9040ce0
440f560
1b3d88e
ace5c0f
7787df6
27753e2
c898201
fb3088a
4956e88
7b29c47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,8 +19,30 @@ If all you want is a unique ID, you should probably call :func:`uuid1` or | |
a UUID containing the computer's network address. :func:`uuid4` creates a | ||
random UUID. | ||
|
||
Depending on support from the underlying platform, :func:`uuid1` may or may | ||
not return a "safe" UUID. A safe UUID is one which is generated using | ||
synchronization methods that ensure no two processes can obtain the same | ||
UUID. All instances of :class:`UUID` have an :attr:`is_safe` attribute | ||
which relays any information about the UUID's safety, using this enumeration: | ||
|
||
.. class:: UUID(hex=None, bytes=None, bytes_le=None, fields=None, int=None, version=None) | ||
.. class:: SafeUUID | ||
|
||
.. attribute:: SafeUUID.safe | ||
|
||
The UUID was generated by the platform in a multiprocessing-safe way. | ||
|
||
.. attribute:: SafeUUID.unsafe | ||
|
||
The UUID was not generated in a multiprocessing-safe way. | ||
|
||
.. attribute:: SafeUUID.unknown | ||
|
||
The platform does not provide information on whether the UUID was generated | ||
safely or not. | ||
|
||
.. versionadded:: 3.7 | ||
|
||
.. class:: UUID(hex=None, bytes=None, bytes_le=None, fields=None, int=None, version=None, *, is_safe=SafeUUID.unknown) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, the body of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
|
||
Create a UUID from either a string of 32 hexadecimal digits, a string of 16 | ||
bytes as the *bytes* argument, a string of 16 bytes in little-endian order as | ||
|
@@ -120,6 +142,13 @@ random UUID. | |
The UUID version number (1 through 5, meaningful only when the variant is | ||
:const:`RFC_4122`). | ||
|
||
.. attribute:: UUID.is_safe | ||
|
||
An enumeration of :class:`SafeUUID` which indicates whether the platform | ||
generated the UUID in a multiprocessing-safe way. | ||
|
||
.. versionadded:: 3.7 | ||
|
||
The :mod:`uuid` module defines the following functions: | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -340,6 +340,46 @@ def test_uuid1(self): | |
equal(((u.clock_seq_hi_variant & 0x3f) << 8) | | ||
u.clock_seq_low, 0x3fff) | ||
|
||
@unittest.skipUnless(importable('ctypes'), 'requires ctypes') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: I'd flip the order of the decorators. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
@unittest.skipUnless(uuid._uuid_generate_time.restype is not None, | ||
'requires uuid_generate_time_safe(3)') | ||
def test_uuid1_safe(self): | ||
u = uuid.uuid1() | ||
# uuid_generate_time_safe() may return 0 or -1 but what it returns is | ||
# dependent on the underlying platform support. At least it cannot be | ||
# unknown (unless I suppose the platform is buggy). | ||
self.assertNotEqual(u.is_safe, uuid.SafeUUID.unknown) | ||
|
||
@unittest.skipUnless(importable('ctypes'), 'requires ctypes') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a suggestion: Using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've played a bit with |
||
def test_uuid1_unknown(self): | ||
# Even if the platform has uuid_generate_time_safe(), let's mock it to | ||
# be uuid_generate_time() and ensure the safety is unknown. | ||
with unittest.mock.patch.object(uuid._uuid_generate_time, | ||
'restype', None): | ||
u = uuid.uuid1() | ||
self.assertEqual(u.is_safe, uuid.SafeUUID.unknown) | ||
|
||
@unittest.skipUnless(importable('ctypes'), 'requires ctypes') | ||
def test_uuid1_is_safe(self): | ||
with unittest.mock.patch.object(uuid._uuid_generate_time, | ||
'restype', lambda x: 0): | ||
u = uuid.uuid1() | ||
self.assertEqual(u.is_safe, uuid.SafeUUID.safe) | ||
|
||
@unittest.skipUnless(importable('ctypes'), 'requires ctypes') | ||
def test_uuid1_is_unsafe(self): | ||
with unittest.mock.patch.object(uuid._uuid_generate_time, | ||
'restype', lambda x: -1): | ||
u = uuid.uuid1() | ||
self.assertEqual(u.is_safe, uuid.SafeUUID.unsafe) | ||
|
||
@unittest.skipUnless(importable('ctypes'), 'requires ctypes') | ||
def test_uuid1_bogus_return_value(self): | ||
with unittest.mock.patch.object(uuid._uuid_generate_time, | ||
'restype', lambda x: 3): | ||
u = uuid.uuid1() | ||
self.assertEqual(u.is_safe, uuid.SafeUUID.unknown) | ||
|
||
def test_uuid3(self): | ||
equal = self.assertEqual | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,6 +46,9 @@ | |
|
||
import os | ||
|
||
from enum import Enum | ||
|
||
|
||
__author__ = 'Ka-Ping Yee <ping@zesty.ca>' | ||
|
||
RESERVED_NCS, RFC_4122, RESERVED_MICROSOFT, RESERVED_FUTURE = [ | ||
|
@@ -55,7 +58,14 @@ | |
int_ = int # The built-in int type | ||
bytes_ = bytes # The built-in bytes type | ||
|
||
class UUID(object): | ||
|
||
class SafeUUID(Enum): | ||
safe = 0 | ||
unsafe = -1 | ||
unknown = None | ||
|
||
|
||
class UUID: | ||
"""Instances of the UUID class represent UUIDs as specified in RFC 4122. | ||
UUID objects are immutable, hashable, and usable as dictionary keys. | ||
Converting a UUID to a string with str() yields something in the form | ||
|
@@ -104,7 +114,8 @@ class UUID(object): | |
""" | ||
|
||
def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None, | ||
int=None, version=None): | ||
int=None, version=None, | ||
*, is_safe=SafeUUID.unknown): | ||
r"""Create a UUID from either a string of 32 hexadecimal digits, | ||
a string of 16 bytes as the 'bytes' argument, a string of 16 bytes | ||
in little-endian order as the 'bytes_le' argument, a tuple of six | ||
|
@@ -128,6 +139,10 @@ def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None, | |
be given. The 'version' argument is optional; if given, the resulting | ||
UUID will have its variant and version set according to RFC 4122, | ||
overriding the given 'hex', 'bytes', 'bytes_le', 'fields', or 'int'. | ||
|
||
is_safe is an enum exposed as an attribute on the instance. It | ||
indicates whether the UUID has been generated in a way that is safe | ||
for multiprocessing applications, via uuid_generate_time_safe(3). | ||
""" | ||
|
||
if [hex, bytes, bytes_le, fields, int].count(None) != 4: | ||
|
@@ -182,6 +197,7 @@ def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None, | |
int &= ~(0xf000 << 64) | ||
int |= version << 76 | ||
self.__dict__['int'] = int | ||
self.__dict__['is_safe'] = is_safe | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you want to assert the type of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure it's worth type checking |
||
|
||
def __eq__(self, other): | ||
if isinstance(other, UUID): | ||
|
@@ -472,10 +488,17 @@ def _netbios_getnode(): | |
for libname in _libnames: | ||
try: | ||
lib = ctypes.CDLL(ctypes.util.find_library(libname)) | ||
except Exception: | ||
except Exception: # pragma: nocover | ||
continue | ||
if hasattr(lib, 'uuid_generate_time'): | ||
# Try to find the safe variety first. | ||
if hasattr(lib, 'uuid_generate_time_safe'): | ||
_uuid_generate_time = lib.uuid_generate_time_safe | ||
# int uuid_generate_time_safe(uuid_t out); | ||
break | ||
elif hasattr(lib, 'uuid_generate_time'): # pragma: nocover | ||
_uuid_generate_time = lib.uuid_generate_time | ||
# void uuid_generate_time(uuid_t out); | ||
_uuid_generate_time.restype = None | ||
break | ||
del _libnames | ||
|
||
|
@@ -566,8 +589,12 @@ def uuid1(node=None, clock_seq=None): | |
# use UuidCreate here because its UUIDs don't conform to RFC 4122). | ||
if _uuid_generate_time and node is clock_seq is None: | ||
_buffer = ctypes.create_string_buffer(16) | ||
_uuid_generate_time(_buffer) | ||
return UUID(bytes=bytes_(_buffer.raw)) | ||
safely_generated = _uuid_generate_time(_buffer) | ||
try: | ||
is_safe = SafeUUID(safely_generated) | ||
except ValueError: | ||
is_safe = SafeUUID.unknown | ||
return UUID(bytes=bytes_(_buffer.raw), is_safe=is_safe) | ||
|
||
global _last_timestamp | ||
import time | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is the current style in uuid.rst, but it might be better use the following style in new documentation:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that looks nice, thanks. The one problem is that
SafeUUID
still gets labeled as a class in the HTML. Okay, technically it is a class, but it would be nice to have anenum::
markup. I grep'd around in the source and couldn't find any such examples, so I don't know if we support that yet.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. We don't have an enum directive now, but it should be easy to add one. You can add me to nosy list if you create an issue on bugs.p.o :)