Repository navigation
[df] Avoid side-effects if external package interacts with distribute… - #23164
Merged
Merged
Conversation
…d module at exit time The distributed RDataFrame Python module raises a warning to let users know it has been moved out of the `Experimental` namespace. This was implemented by creating a dummy Python class type that injects the warning at `__getattribute__` time and using that class as the class of the injected distributed module when the user accesses it via `Experimental`. That implementation may have undesired interactions in extreme edge-cases. For example, if a third-party library accesses available Python modules during the session and inspects them, the distributed Python module warning could be triggered. One notable example are the XRootD Python bindings. The `client` module of this package registers a function with `atexit`. At exit time, that function accesses all objects available to the garbage collector, which includes all Python modules (even external to XRootD itself). See the implementation at: https://github.com/xrootd/xrootd/blob/v6.1.1/python/libs/client/finalize.py While accessing all these objects, XRootD is also accessing ROOT modules if ROOT was imported in the same application. As such, the dummy class behaviour explained above kicked in. This commit proposes to change the implementation that raises the warning to use a mechanism based on function decorators. The downside is a few more lines of code: every function supposed to be accessed publicly of the distributed module must be decorated so that the warning is raised before the function runs if the user is accessing it via `Experimental`. The upside is avoiding the scenario described above and having the warning raised right at the point of calling the function rather than slightly earlier during module attribute access. Co-authored-by: Silia Taider <silia.taider@cern.ch>
Test Results 23 files 23 suites 3d 15h 33m 12s ⏱️ For more details on these failures, see this check. Results for commit 4b8ae87. |
dpiparo
approved these changes
Aug 28, 2026
Member
|
to be backported to 6.40 for 6.40.06? |
Member
Author
|
/backport to 6.40 |
|
Preparing to backport PR #23164 to branch 6.40 requested by vepadulano |
|
Something went wrong with the creation of the PR to backport to 6.40: @vepadulano please see the logs |
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.
…d module at exit time
The distributed RDataFrame Python module raises a warning to let users know it has been moved out of the
Experimentalnamespace. This was implemented by creating a dummy Python class type that injects the warning at__getattribute__time and using that class as the class of the injected distributed module when the user accesses it viaExperimental.That implementation may have undesired interactions in extreme edge-cases. For example, if a third-party library accesses available Python modules during the session and inspects them, the distributed Python module warning could be triggered. One notable example are the XRootD Python bindings. The
clientmodule of this package registers a function withatexit. At exit time, that function accesses all objects available to the garbage collector, which includes all Python modules (even external to XRootD itself). See the implementation at:https://github.com/xrootd/xrootd/blob/v6.1.1/python/libs/client/finalize.py
While accessing all these objects, XRootD is also accessing ROOT modules if ROOT was imported in the same application. As such, the dummy class behaviour explained above kicked in.
This commit proposes to change the implementation that raises the warning to use a mechanism based on function decorators. The downside is a few more lines of code: every function supposed to be accessed publicly of the distributed module must be decorated so that the warning is raised before the function runs if the user is accessing it via
Experimental. The upside is avoiding the scenario described above and having the warning raised right at the point of calling the function rather than slightly earlier during module attribute access.This Pull request:
Changes or fixes:
Fixes #23072
Checklist:
This PR fixes #