Skip to content

Comments

Add context to unregistered task name to error context#4471

Merged
konstin merged 1 commit intomainfrom
konsti/unregistered-context
Jun 24, 2024
Merged

Add context to unregistered task name to error context#4471
konstin merged 1 commit intomainfrom
konsti/unregistered-context

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
@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