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

Show clearer grpc errors, as well as a custom failure for sensor timeouts in particular since those are so common #11576

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

gibsondan
Copy link
Member

Summary:

  • surface the error code instead of burying it in the nested cause error
  • for timeouts, include the original timeout (and that it timed out)
  • For sensors, include how you might fix it

Summary & Motivation

How I Tested These Changes

@vercel
Copy link

vercel bot commented Jan 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated
dagit-storybook ⬜️ Ignored (Inspect) Jan 9, 2023 at 11:20PM (UTC)
dagster ⬜️ Ignored (Inspect) Jan 9, 2023 at 11:20PM (UTC)

if e.code() == grpc.StatusCode.DEADLINE_EXCEEDED:
raise DagsterUserCodeUnreachableError(
custom_timeout_message
or f"User code query timed out due to taking longer than {timeout} seconds to complete."
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: user code -> code location server

Suggested change
or f"User code query timed out due to taking longer than {timeout} seconds to complete."
or f"The query to the code location server timed out due to taking longer than {timeout} seconds to complete."

…outs in particular since those are so common

Summary:
- surface the error code instead of burying it in the nested cause error
- for timeouts, include the original timeout (and that it timed out)
- For sensors, include how you might fix it
@gibsondan gibsondan merged commit 5f18862 into master Jan 10, 2023
@gibsondan gibsondan deleted the sensorhint branch January 10, 2023 01:54
AlexanderVR pushed a commit to AlexanderVR/dagster that referenced this pull request Jan 10, 2023
…outs in particular since those are so common (dagster-io#11576)

Summary:
- surface the error code instead of burying it in the nested cause error
- for timeouts, include the original timeout (and that it timed out)
- For sensors, include how you might fix it

### Summary & Motivation

### How I Tested These Changes
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