Skip to content

Add more user-friendly error message upon async def remote task#13915

Merged
simon-mo merged 6 commits intoray-project:masterfrom
kathryn-zhou:async_error
Feb 5, 2021
Merged

Add more user-friendly error message upon async def remote task#13915
simon-mo merged 6 commits intoray-project:masterfrom
kathryn-zhou:async_error

Conversation

@kathryn-zhou
Copy link
Copy Markdown
Contributor

@kathryn-zhou kathryn-zhou commented Feb 4, 2021

Why are these changes needed?

This PR gives the user an actionable error message when they try to use async def to create a remote task. The previous error message was not very user-friendly and did not provide actions to fix the error.

Related issue number

Closes #13687

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@kathryn-zhou kathryn-zhou requested a review from simon-mo February 4, 2021 22:40
@kathryn-zhou kathryn-zhou changed the title Add more user-friendly Add more user-friendly error message for async remote task Feb 4, 2021
@kathryn-zhou kathryn-zhou changed the title Add more user-friendly error message for async remote task Add more user-friendly error message upon async def remote task Feb 4, 2021
Copy link
Copy Markdown
Contributor

@simon-mo simon-mo left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +181 to +187
@ray.remote
def wrapper():
import asyncio
asyncio.get_event_loop().run_until_complete(f())

async def f():
pass No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we swap the order of these functions? make it easier to read (so f isn't undefined when inside wrapper

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yep makes sense, done

@simon-mo simon-mo merged commit 982c606 into ray-project:master Feb 5, 2021
fishbone added a commit to fishbone/ray that referenced this pull request Feb 16, 2021
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.

[Core] Add more user-friendly error message upon async def remote task

2 participants