fix: open job-instance.pickle in binary mode in LSF contrib (#3330) - #3443
Open
1102Aryan wants to merge 3 commits into
Open
fix: open job-instance.pickle in binary mode in LSF contrib (#3330)#34431102Aryan wants to merge 3 commits into
1102Aryan wants to merge 3 commits into
Conversation
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
lsf_runner.pyopenedjob-instance.picklein text mode ("r"), which fails onPython 3 because
pickle.load()requires a binary stream. This PR:luigi/contrib/lsf_runner.py: open the pickle file with"rb"instead of"r"luigi/contrib/lsf.py: write the pickle file with"wb"instead of"w"in_dump(), and use bytes literals in the__main__-module.replace()call,since
pickle.dumps()returns bytes on Python 3Motivation and Context
Fixes #3330. Any LSF task run through
lsf_runner.pyon Python 3 crashes withTypeError: a bytes-like object is required, not 'str'(orUnicodeDecodeError,depending on pickle protocol and platform) before the job's
work()method can run.Have you tested this? If so, how?
I reproduced the reported error on master with a script that pickles a job object
and calls
lsf_runner.do_work_on_compute_node()— it fails exactly as described inthe issue. With this fix, the same round-trip succeeds and the job's
work()methodruns, including the
__module__ == "__main__"dump path. The existing LSF tests(
test/contrib/lsf_test.py) and ruff lint checks also pass.I have also included a unit test that round trips the pickle through
_dump()andlst_runner.do_work_on_compute_node()