Skip to content
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

Document how we serialize metadata and warn when it is pickled #2156

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

poodlewars
Copy link
Collaborator

No description provided.

@poodlewars poodlewars added documentation Improvements or additions to documentation enhancement New feature or request labels Jan 30, 2025
@@ -78,6 +79,9 @@ def check_is_utc_if_newer_pandas(*args, **kwargs):
NormalizedInput = NamedTuple("NormalizedInput", [("item", NPDDataFrame), ("metadata", NormalizationMetadata)])


PICKLED_METADATA_LOGLEVEL = LogLevel.DEBUG if os.getenv("PICKLED_METADATA_LOGLEVEL_DEBUG") else LogLevel.WARN
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this follow the format of our other env vars? And maybe our other logging env vars?

@alexowens90
Copy link
Collaborator

I know you haven't changed it, but this is obviously wrong:
https://github.com/man-group/ArcticDB/blame/master/python/arcticdb/version_store/_normalization.py#L1049-L1055
Changed here:
4b29af8#diff-8d6759b6a2a11f86f70ee321e943d6c8e7898d4419ed281ad5a7358426a49a9a
Not sure if this means we can no longer unpickle data pickled with Python 2?

@poodlewars
Copy link
Collaborator Author

I'm going to merge this as I've addressed the comments and Alex is off now. I've raised some logical changes in #2160

@poodlewars poodlewars merged commit 734e50a into master Jan 31, 2025
152 checks passed
@poodlewars poodlewars deleted the metadata-pickling-docs branch January 31, 2025 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants