Implement a mechanism for registering and marking functions as thread-unsafe #37
Conversation
ngoldbaum
left a comment
ngoldbaum
left a comment
There was a problem hiding this comment.
LGTM, mostly language suggestions and one small API suggestion. The test makes it clear that this is working as expected.
I didn't try messing with the SciPy tests and didn't verify if this fixes the issue Ralf originally had, but it looks like it should.
src/pytest_run_parallel/utils.py
Outdated
| if len(node.targets) == 1: | ||
| name_node = node.targets[0] | ||
| value_node = node.value | ||
| if getattr(name_node, "id", None) == "__thread_unsafe__": |
There was a problem hiding this comment.
| if getattr(name_node, "id", None) == "__thread_unsafe__": | |
| if hasattr(name_node, "id", None) == "__thread_unsafe__": |
That way the default is the same as if the attribute isn't defined at all and it doesn't matter what the value is.
There was a problem hiding this comment.
I think hasattr does not allow for 3 arguments, whereas getattr does.
There was a problem hiding this comment.
Ah yeah, sorry. Not literally that diff!
There was a problem hiding this comment.
Unless you refer to marking the test as thread-unsafe by just defining __thread_unsafe__
There was a problem hiding this comment.
Yeah, that’s exactly right. I’m saying you should only need to define the attribute and it can have any value.
If you want to keep the boolean flag that’s fine, but I think it should be spelled __thread_safe__ = False so the default is True. People have an easier time thinking about boolean flags that default to True.
README.rst
Outdated
| module.submodule2.func2 | ||
| ... | ||
| Also, if you define a `__thread_unsafe__` attribute on a function that is |
There was a problem hiding this comment.
| Also, if you define a `__thread_unsafe__` attribute on a function that is | |
| Also, if you set `__thread_safe__ = False` as an attribute on a function that is |
Co-authored-by: Nathan Goldbaum <[email protected]>
andfoy
force-pushed
the
extend_skip_functions
branch
from
58bf49a to
85d281d
Compare
|
Thanks, let's do followups if for some reason SciPy needs additional changes. |
Fixes #35
This PR adds two new mechanisms to register custom functions whose invocation should cause a test to be executed in a single thread.
The first works by setting the variable
__thread_unsafe__toTrue, while the second one relies on static values provided via the INI configuration file. On both cases, the functions will be checked up to two levels in the call-stack (derived from the AST).cc @rgommers