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

close client in sync test_actor tests #6459

Merged
merged 1 commit into from
May 26, 2022

Conversation

graingert
Copy link
Member

@graingert graingert commented May 26, 2022

refs #6453

Not sure if this is a fix, but closing the client explicitly usually helps with unclosed comms

  • Tests added / passed
  • Passes pre-commit run --all-files

@mrocklin
Copy link
Member

Looks good to me. Merging after CI. Any reason to keep this as a draft?

@graingert graingert marked this pull request as ready for review May 26, 2022 13:03
@graingert
Copy link
Member Author

Looks good to me. Merging after CI. Any reason to keep this as a draft?

was waiting for tests, but you're right just moving the client to the with shouldn't need to worry

@mrocklin
Copy link
Member

To me Draft state means "the author doesn't yet want this to be merged" and non-green tests means "the robots don't yet want this to be merged". If you think that this is likely going to be ok, pending the review of the bots, then I think it's ok to drop the Draft status. That's just how my thinking works though. Others may have other opinions.

@github-actions
Copy link
Contributor

Unit Test Results

       15 files  ±0         15 suites  ±0   6h 19m 44s ⏱️ - 11m 39s
  2 813 tests ±0    2 733 ✔️ ±0    79 💤 ±0  1 ±0 
20 854 runs  ±0  19 920 ✔️  - 3  933 💤 +3  1 ±0 

For more details on these failures, see this check.

Results for commit 411dc79. ± Comparison against base commit 5e9e97f.

@mrocklin
Copy link
Member

Both failures are test_stress_scatter_death and unrelated. Merging. Thanks for the testing cleanup @graingert !

@mrocklin mrocklin merged commit e0ea5df into dask:main May 26, 2022
@graingert graingert deleted the close-client-in-sync-test-actor-tests branch May 26, 2022 14:43
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.

2 participants