Skip to content

close client in sync test_actor tests#6459

Merged
mrocklin merged 1 commit intodask:mainfrom
graingert:close-client-in-sync-test-actor-tests
May 26, 2022
Merged

close client in sync test_actor tests#6459
mrocklin merged 1 commit intodask:mainfrom
graingert:close-client-in-sync-test-actor-tests

Conversation

@graingert
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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