Conversation
|
In principle this seems fine to me |
|
Failing tests seem to be unrelated and have flaked before. |
fjetter
left a comment
There was a problem hiding this comment.
Code looks good. One small question about a test but otherwise we're good to merge
| ptp = ProfileServer(s) | ||
| assert "disabled" in ptp.subtitle.text | ||
| start = time() | ||
| await asyncio.sleep(1) |
There was a problem hiding this comment.
Do you actually need to wait for a second? with the configured intervals/cycles I would expect this to be a fraction of a second such that test runtime is lower
There was a problem hiding this comment.
I wanted to be on the safe side wrt. CI randomly suspending execution for some time. Writing this down, I realize that this would cause a false negative flake, i.e. this test would not fail even though it is broken, and it should still fail whenever CI does not suspend execution, very much including local machines. So I guess it's okay to reduce the sleep less and have this test occasionally pass on CI should it ever break.
Closes #6075
This PR adds the
distributed.worker.profile.enabledconfig option which defaults toTrue. If set toFalse, profiling is deactivated. For any tests that transitively make use of thedistributed.utils_test._reconfigure()context manager, profiling is now deactivated unless explicitly requested./profileand/profile-serverendpoints now show a message and have disabled buttons if profiling is deactivated:/profile/profile-serverOut of Scope:
_reconfigure()a global fixture or generally implement a global mechanism to set some config options per defaultpre-commit run --all-files