-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Added function for deterministic ID for class definitions by hashing. #36793
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 2 commits
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 | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -62,6 +62,7 @@ | |||||||||||||
| import dis | ||||||||||||||
| from enum import Enum | ||||||||||||||
| import functools | ||||||||||||||
| import hashlib | ||||||||||||||
| import io | ||||||||||||||
| import itertools | ||||||||||||||
| import logging | ||||||||||||||
|
|
@@ -98,7 +99,7 @@ | |||||||||||||
| _DYNAMIC_CLASS_TRACKER_BY_CLASS = weakref.WeakKeyDictionary() | ||||||||||||||
| _DYNAMIC_CLASS_TRACKER_BY_ID = weakref.WeakValueDictionary() | ||||||||||||||
| _DYNAMIC_CLASS_STATE_TRACKER_BY_CLASS = weakref.WeakKeyDictionary() | ||||||||||||||
| _DYNAMIC_CLASS_TRACKER_LOCK = threading.Lock() | ||||||||||||||
| _DYNAMIC_CLASS_TRACKER_LOCK = threading.RLock() | ||||||||||||||
|
|
||||||||||||||
| PYPY = platform.python_implementation() == "PyPy" | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -168,6 +169,7 @@ class CloudPickleConfig: | |||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| DEFAULT_CONFIG = CloudPickleConfig() | ||||||||||||||
| _GENERATING_SENTINEL = object() | ||||||||||||||
| builtin_code_type = None | ||||||||||||||
| if PYPY: | ||||||||||||||
| # builtin-code objects only exist in pypy | ||||||||||||||
|
|
@@ -179,10 +181,21 @@ class CloudPickleConfig: | |||||||||||||
| def _get_or_create_tracker_id(class_def, id_generator): | ||||||||||||||
| with _DYNAMIC_CLASS_TRACKER_LOCK: | ||||||||||||||
| class_tracker_id = _DYNAMIC_CLASS_TRACKER_BY_CLASS.get(class_def) | ||||||||||||||
| if class_tracker_id is _GENERATING_SENTINEL and id_generator: | ||||||||||||||
| raise RuntimeError( | ||||||||||||||
| f"Recursive ID generation detected for {class_def}. " | ||||||||||||||
| f"The id_generator cannot recursively request an ID for the same class." | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| if class_tracker_id is None and id_generator is not None: | ||||||||||||||
| class_tracker_id = id_generator(class_def) | ||||||||||||||
| _DYNAMIC_CLASS_TRACKER_BY_CLASS[class_def] = class_tracker_id | ||||||||||||||
| _DYNAMIC_CLASS_TRACKER_BY_ID[class_tracker_id] = class_def | ||||||||||||||
| _DYNAMIC_CLASS_TRACKER_BY_CLASS[class_def] = _GENERATING_SENTINEL | ||||||||||||||
| try: | ||||||||||||||
| class_tracker_id = id_generator(class_def) | ||||||||||||||
| _DYNAMIC_CLASS_TRACKER_BY_CLASS[class_def] = class_tracker_id | ||||||||||||||
| _DYNAMIC_CLASS_TRACKER_BY_ID[class_tracker_id] = class_def | ||||||||||||||
| except: | ||||||||||||||
| _DYNAMIC_CLASS_TRACKER_BY_CLASS.pop(class_def, None) | ||||||||||||||
| raise | ||||||||||||||
| return class_tracker_id | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
|
|
@@ -1720,3 +1733,10 @@ def dumps( | |||||||||||||
|
|
||||||||||||||
| # Backward compat alias. | ||||||||||||||
| CloudPickler = Pickler | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def hash_dynamic_classdef(classdef): | ||||||||||||||
|
Contributor
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.
Collaborator
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. It will be used by flume runner. It is tested in the cloud pickle repo. Conflicts would be bad, should we try to detect?
Collaborator
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. Although conflicts should result in unpickling ereor
Collaborator
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. Although the original uuid approach can also cause collision?
Contributor
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'd worry if the errors are silenced.
Contributor
Author
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.
|
||||||||||||||
| """Generates a deterministic ID by hashing the pickled class definition.""" | ||||||||||||||
| hexidgest = hashlib.sha256( | ||||||||||||||
| dumps(classdef, config=CloudPickleConfig(id_generator=None))).hexdigest() | ||||||||||||||
| return hexidgest | ||||||||||||||
|
||||||||||||||
| hexidgest = hashlib.sha256( | |
| dumps(classdef, config=CloudPickleConfig(id_generator=None))).hexdigest() | |
| return hexidgest | |
| hexdigest = hashlib.sha256( | |
| dumps(classdef, config=CloudPickleConfig(id_generator=None))).hexdigest() | |
| return hexdigest |
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.
Using a bare
except:can catch more exceptions than intended, such asSystemExitorKeyboardInterrupt, which can hide bugs or make it harder to stop the program. It's better to be more specific and catchExceptioninstead to avoid unintentionally swallowing system-level exceptions.