Relax check_version_info to check for bytecode compatibility #41373
Conversation
This file contains 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
geofft
requested review from
ericl,
richardliaw,
rkooo567 and
jjyao
as code owners
geofft
force-pushed
the
check-magic-number
branch
2 times, most recently
from
39f41bb
to
10443bb
Compare
As discussed in ray-project#3988, the intention of check_version_info is to ensure that cloudpickle can transfer objects across the two Python interpreters. Because cloudpickle serializes Python bytecode, it is known to be incompatible across minor versions (e.g., 3.11 to 3.12) but should be compatible across patch versions (e.g., 3.12.0 to 3.12.1). The intention of the CPython team is that bytecode should be compatible within patch releases to the same minor version. This was last broken in 3.5.3, which was regarded as a mistake: see the commit message of python/cpython@93602e3 (in 3.5.10), which implies that it is reasonable to "rel[y] on pre-cached bytecode remaining valid across maintenance releases." The constant importlib.util.MAGIC_NUMBER is used by Python to identify bytecode compatibility (it is stored in .pyc files and checked when they are loaded). The unwanted change in 3.5.3 bumped MAGIC_NUMBER, but since then, it has only changed between minor versions. See the long comment in CPython's Lib/importlib/_bootstrap_external.py for a history of the changes. So, the practical effect of checking MAGIC_NUMBER will generally be to enforce that nodes run the same minor version of Python, but if some future patch release unexpectedly does break bytecode compatibility, checking MAGIC_NUMBER will account for that. Signed-off-by: Geoffrey Thomas <[email protected]>
geofft
force-pushed
the
check-magic-number
branch
from
10443bb
to
1d53e61
Compare
|
Your explanation makes sense to me. But I think we need to get some confirmation from cloudpickle devs saying that checking MAGIC_NUMBER is enough? cc @pcmoritz |
anyscalesam
added
triage
jjyao
added
P1
|
@jjyao |
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.
As discussed in #3988, the intention of check_version_info is to ensure that cloudpickle can transfer objects across the two Python interpreters. Because cloudpickle serializes Python bytecode, it is known to be incompatible across minor versions (e.g., 3.11 to 3.12) but should be compatible across patch versions (e.g., 3.12.0 to 3.12.1).
The intention of the CPython team is that bytecode should be compatible within patch releases to the same minor version. This was last broken in 3.5.3, which was regarded as a mistake: see the commit message of python/cpython@93602e3 (in 3.5.10), which implies that it is reasonable to "rel[y] on pre-cached bytecode remaining valid across maintenance releases."
The constant importlib.util.MAGIC_NUMBER is used by Python to identify bytecode compatibility (it is stored in .pyc files and checked when they are loaded). The unwanted change in 3.5.3 bumped MAGIC_NUMBER, but since then, it has only changed between minor versions. See the long comment in CPython's Lib/importlib/_bootstrap_external.py for a history of the changes.
So, the practical effect of checking MAGIC_NUMBER will generally be to enforce that nodes run the same minor version of Python, but if some future patch release unexpectedly does break bytecode compatibility, checking MAGIC_NUMBER will account for that.
Why are these changes needed?
This allows using interpreters at the same minor version but a different patch version, as requested in #3988.
Related issue number
Closes #3988.
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.blackby handmethod in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.