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.

Sign up for GitHub

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

Jump to bottom

Properly await async method client.wait_for_workers #8558

Merged
merged 2 commits into from Dec 7, 2022
Merged

Properly await async method client.wait_for_workers #8558

merged 2 commits into from Dec 7, 2022

Conversation

mrocklin
Copy link
Contributor

@mrocklin mrocklin commented Dec 6, 2022

Previously this async method was called but never awaited, resulting in a confusing message:

RuntimeWarning: coroutine 'Client._wait_for_workers' was never awaited
  client.wait_for_workers(n_workers)

Now we await it.

trivialfis
trivialfis approved these changes Dec 7, 2022
View reviewed changes
Copy link
Member

@trivialfis trivialfis left a comment

Choose a reason for hiding this comment

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

Thank you for the fix! Will port it to 1.7.2.

@trivialfis trivialfis mentioned this pull request Dec 7, 2022
Closed
9 tasks
@trivialfis
Copy link
Member

@mrocklin Hi, I will push a type: ignore to workaround the mypy check for now. But I think the annotation for wait_for_workers is inaccurate and it should not return None.

@mrocklin
Copy link
Contributor Author

mrocklin commented Dec 7, 2022

Thank you @trivialfis . Please let me know if there is anything more that I should do here.

@trivialfis trivialfis merged commit b7ffdcd into dmlc:master Dec 7, 2022
@trivialfis
Copy link
Member

Thank you for the fix!

trivialfis added a commit to trivialfis/xgboost that referenced this pull request Dec 7, 2022
@mrocklin @trivialfis
Properly await async method client.wait_for_workers (dmlc#8558)
74a1fde 
* Properly await async method client.wait_for_workers

* ignore mypy error.

Co-authored-by: jiamingy <jm.yuan@outlook.com>
trivialfis added a commit that referenced this pull request Dec 7, 2022
@trivialfis @mrocklin
Properly await async method client.wait_for_workers (#8558) (#8567)
a980e10 
* Properly await async method client.wait_for_workers

* ignore mypy error.

Co-authored-by: jiamingy <jm.yuan@outlook.com>

Co-authored-by: Matthew Rocklin <mrocklin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivialfis trivialfis trivialfis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mrocklin @trivialfis