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

Add context to unregistered task name to error context #4471

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

konstin
Copy link
Member

@konstin konstin commented Jun 24, 2024

I caused this error during development and having the name of the task on it is helpful for debugging.

Split out from #4435

@konstin konstin added the internal A refactor or improvement that is not user-facing label Jun 24, 2024
@konstin konstin enabled auto-merge (squash) June 24, 2024 13:21
@charliermarsh
Copy link
Member

Is the dist ID enough to distinguish the "task"?

@charliermarsh
Copy link
Member

I have also done something like this so very much in favor.

.wait_blocking(&version_id)
.ok_or(ResolveError::Unregistered)?;
.wait_blocking(&dist.version_id())
.ok_or(ResolveError::Unregistered(dist.version_id().to_string()))?;
Copy link
Member

Choose a reason for hiding this comment

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

Should these now be in a closure to avoid allocation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, will change

@konstin konstin disabled auto-merge June 24, 2024 13:26
@konstin
Copy link
Member Author

konstin commented Jun 24, 2024

Is the dist ID enough to distinguish the "task"?

For me the problem was package name vs. url so this was sufficient

@konstin konstin force-pushed the konsti/unregistered-context branch from 32d72f8 to 6c4a50c Compare June 24, 2024 14:09
I caused this error during development and having the name of the task on it is helpful for debugging.
@konstin konstin force-pushed the konsti/unregistered-context branch from 6c4a50c to bb48d69 Compare June 24, 2024 14:35
@konstin konstin enabled auto-merge (squash) June 24, 2024 14:35
@konstin konstin merged commit 40f8526 into main Jun 24, 2024
47 checks passed
@konstin konstin deleted the konsti/unregistered-context branch June 24, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal A refactor or improvement that is not user-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants