bito-code-review[bot] commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4655426574
<!-- Bito Reply -->
The architectural review finding is correct. The current implementation of
`_stub` permanently modifies `sys.modules`, which causes cross-test
contamination. To resolve this, you should save the original `sys.modules`
state and restore it after the module is loaded, or use a context manager to
handle the temporary stubs.
Below is a minimal fix using a context manager to ensure `sys.modules` is
restored after the module is loaded:
```python
import sys
from contextlib import contextmanager
@contextmanager
def temporary_stubs(stubs):
original = {name: sys.modules.get(name) for name in stubs}
try:
for name, mod in stubs.items():
sys.modules[name] = mod
yield
finally:
for name, original_mod in original.items():
if original_mod is None:
sys.modules.pop(name, None)
else:
sys.modules[name] = original_mod
# Usage:
# stubs = {"celery": _stub("celery"), ...}
# with temporary_stubs(stubs):
# _spec.loader.exec_module(_mod)
```
Would you like me to fetch all other comments on this PR to validate and
implement fixes for them as well?
**tests/unit_tests/tasks/test_get_current_user.py**
```
@contextmanager
def temporary_stubs(stubs):
original = {name: sys.modules.get(name) for name in stubs}
try:
for name, mod in stubs.items():
sys.modules[name] = mod
yield
finally:
for name, original_mod in original.items():
if original_mod is None:
sys.modules.pop(name, None)
else:
sys.modules[name] = original_mod
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
