Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle failing plugin.close() calls during scheduler shutdown #6450

Merged
merged 4 commits into from
May 26, 2022

Conversation

mrocklin
Copy link
Member

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented May 26, 2022

Unit Test Results

       15 files  ±  0         15 suites  ±0   6h 19m 36s ⏱️ - 1m 13s
  2 814 tests +  2    2 733 ✔️ +1    81 💤 +  2  0  - 1 
20 861 runs  +14  19 918 ✔️ +3  943 💤 +12  0  - 1 

Results for commit 8971441. ± Comparison against base commit 046ab17.

♻️ This comment has been updated with latest results.

distributed/scheduler.py Outdated Show resolved Hide resolved
distributed/scheduler.py Outdated Show resolved Hide resolved
@mrocklin
Copy link
Member Author

Sure. I was just being lazy. pushed up

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mrocklin. This looks like a reasonable addition. I've left a couple of nice-to-have, but not blocking comments

Comment on lines +236 to +237
assert "123" in out
assert "456" in out
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also be nice to check that the logger exceptions also occurred as expected

try:
await func()
except Exception:
logger.exception("Plugin call failed during scheduler.close")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including the plugin name in this message would be nice because as is we just know that some plugin method failed, but not which plugin

Copy link
Member

@graingert graingert May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logger.exception includes the traceback which should include the plugin class name, although it could be a base class that fails

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks for pointing that out @graingert

@pentschev
Copy link
Member

rerun tests

Note: rerunning gpuCI tests since those errors should be fixed by #6434

@jrbourbeau jrbourbeau changed the title Handle failing plugin.close calls during scheduler shutdown Handle failing plugin.close() calls during scheduler shutdown May 26, 2022
@jrbourbeau jrbourbeau merged commit 9a70a53 into dask:main May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants