Make CollectionLookupError returns more precise#35011
Merged
ggevay merged 4 commits intoMaterializeInc:mainfrom Feb 19, 2026
Merged
Make CollectionLookupError returns more precise#35011ggevay merged 4 commits intoMaterializeInc:mainfrom
CollectionLookupError returns more precise#35011ggevay merged 4 commits intoMaterializeInc:mainfrom
Conversation
Pre-merge checklist
|
ggevay
force-pushed
the
instance_client-refactoring-2
branch
from
3898271 to
7b0f72e
Compare
Contributor
Author
|
Rebased, ready for review |
teskje
approved these changes
Feb 19, 2026
Contributor
teskje
left a comment
teskje
left a comment
There was a problem hiding this comment.
Lgtm, just minor comments.
Comment on lines
+50
to
+49
| pub enum CollectionLookupError { | ||
| pub enum CollectionFrontiersError { |
Contributor
There was a problem hiding this comment.
I'd keep this named CollectionLookupError, since it's also returned by install_compute_watchset, which is not directly about frontier lookups.
|
|
||
| /// Errors arising during peek watch set installation. | ||
| #[derive(Error, Debug)] | ||
| pub enum WatchSetInstallError { |
Contributor
There was a problem hiding this comment.
I wonder if there is a need for a new error enum if it just reproduces the one from the controller. Does returning the controller error like we did before not work here?
Contributor
Author
There was a problem hiding this comment.
Indeed, removed this and using CollectionLookupError instead.
antiguru
reviewed
Feb 19, 2026
ggevay
force-pushed
the
instance_client-refactoring-2
branch
from
7b0f72e to
60ec7e3
Compare
Contributor
Author
|
TFTR! |
patrickwwbutler
pushed a commit
to patrickwwbutler/materialize
that referenced
this pull request
Feb 19, 2026
This is a refactoring related to the frontend peek sequencing, addressing the second paragraph of MaterializeInc#33713 (comment) + other related discussions with Moritz and Jan. This PR makes many function return types specify more precise error types. This made `CollectionLookupError` used only by `PeekClient`, so it was moved there. Part 2 of 3 from the original MaterializeInc#34839. Depends on MaterializeInc#35010, so only the second commit is new. Edit: Now rebased, the original first commit is on `main`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a refactoring related to the frontend peek sequencing, addressing the second paragraph of #33713 (comment) + other related discussions with Moritz and Jan.
This PR makes many function return types specify more precise error types. This made
CollectionLookupErrorused only byPeekClient, so it was moved there.Part 2 of 3 from the original #34839. Depends on #35010, so only the second commit is new. Edit: Now rebased, the original first commit is on
main.