-
Notifications
You must be signed in to change notification settings - Fork 142
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
Nexus: Handle pickle protocols between python versions #4385
Conversation
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.
Please add exception handling on the pickle5 import and print a message about why it is needed if the import fails.
The mixed python situation is fairly common. e.g. Old system python, user installed recent python.
While tweaking this code, add some error handling on the open call? Or is the file always guaranteed to be there and readable at this point? |
Import error exception is added, please check. File is always guaranteed to be there and readable per nexus workflows. |
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.
LGTM
I tweaked the message since there are many ways to install python modules and pip might not be the relevant one.
Test this please |
Test this please |
Proposed changes
Handles the pickle (https://docs.python.org/3/library/pickle.html) protocol differences that can arise due to using different python versions, (such as using multiple computers having different python versions, but sharing the same filesystem). In the current code, if python 3.8 is used to generate pickle (.p) files, nexus uses protocol 5. If python 3.7 is used to rerun the same script, protocol 5 is not accessible in that python version (https://docs.python.org/3/library/pickle.html#data-stream-format)
What type(s) of changes does this code introduce?
Does this introduce a breaking change?
What systems has this change been tested on?
Andes, Summit
Checklist
Update the following with a yes where the items apply. If you're unsure about any of them, don't hesitate to ask. This is
simply a reminder of what we are going to look for before merging your code.