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

avoid calls to make_current() and make_clear() by using asyncio.run in LoopRunner #7467

Merged
merged 12 commits into from
Jun 16, 2023

Conversation

graingert
Copy link
Member

@graingert graingert commented Jan 11, 2023

Closes #6784

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

@github-actions
Copy link
Contributor

github-actions bot commented Jan 11, 2023

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       20 files  ±  0         20 suites  ±0   11h 51m 48s ⏱️ + 8m 20s
  3 684 tests +  3    3 573 ✔️ +  1     108 💤 ±0  3 +3 
35 630 runs  +30  33 863 ✔️ +30  1 764 💤  - 1  3 +2 

For more details on these failures, see this check.

Results for commit f18f9e7. ± Comparison against base commit 4a0c489.

♻️ This comment has been updated with latest results.

@graingert graingert force-pushed the avoid-calls-to-make-current-make-clear branch 5 times, most recently from be56b65 to 86de36a Compare January 12, 2023 16:59
@graingert graingert force-pushed the avoid-calls-to-make-current-make-clear branch from 86de36a to f121d04 Compare February 1, 2023 14:39
@graingert graingert force-pushed the avoid-calls-to-make-current-make-clear branch from f121d04 to 21c7e40 Compare February 15, 2023 14:03
@graingert graingert marked this pull request as ready for review February 15, 2023 14:03
Copy link
Collaborator

@gjoseph92 gjoseph92 left a comment

Choose a reason for hiding this comment

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

Some questions about thread-safety, but this seems good to me overall.

distributed/deploy/cluster.py Outdated Show resolved Hide resolved
distributed/utils.py Show resolved Hide resolved
distributed/utils.py Show resolved Hide resolved
distributed/utils.py Show resolved Hide resolved
distributed/utils.py Show resolved Hide resolved
distributed/utils.py Outdated Show resolved Hide resolved
Co-authored-by: Gabe Joseph <gjoseph92@gmail.com>
@graingert graingert force-pushed the avoid-calls-to-make-current-make-clear branch from 621adb7 to 5b279be Compare June 4, 2023 08:33
@hendrikmakait hendrikmakait added the needs review Needs review from a contributor. label Jun 5, 2023
@crusaderky crusaderky requested review from crusaderky and removed request for fjetter June 9, 2023 15:34
Copy link
Collaborator

@crusaderky crusaderky left a comment

Choose a reason for hiding this comment

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

Very minor nitpicks only

distributed/deploy/cluster.py Outdated Show resolved Hide resolved
distributed/deploy/cluster.py Show resolved Hide resolved
distributed/utils.py Outdated Show resolved Hide resolved
distributed/utils.py Outdated Show resolved Hide resolved
distributed/utils.py Show resolved Hide resolved
distributed/utils.py Outdated Show resolved Hide resolved
graingert and others added 2 commits June 13, 2023 17:51
Co-authored-by: crusaderky <crusaderky@gmail.com>
Co-authored-by: crusaderky <crusaderky@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Needs review from a contributor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make_current/clear_current are deprecated in Tornado
4 participants