Skip to content

Conversation

@davinci26
Copy link
Member

MuxCache doesn't implement the Fetch method and it returns an error. However MuxCache doesn't have any logic instead it delegates to the appropriate cache based on the request. This means that we can just use the same logic as CreateWatch to delegate to the appropriate cache's Fetch method.

Linear cache doesnt implement Fetch either so this part of the call will fail but the rest of the caches will work as expected.

Additionally it was a bit confusing to debug the issue since the error message resembles quite a bit a gRPC Unimplemented error. So I updated the error message in Linear cache to be more specific.

`MuxCache` doesn't implement the `Fetch` method and it returns an error.
However `MuxCache` doesn't have any logic instead it delegates to the appropriate
cache based on the request. This means that we can just use the same logic as `CreateWatch`
to delegate to the appropriate cache's `Fetch` method.

`Linear` cache doesnt implement `Fetch` either so this part of the call will fail but the rest
of the caches will work as expected.

Additionally it was a bit confusing to debug the issue since the error message resembles quite a
bit a `gRPC Unimplemented` error. So I updated the error message in `Linear` cache to be more specific.

Signed-off-by: sotiris <[email protected]>
@davinci26
Copy link
Member Author

@valerian-roche I see you are pretty active in the repo so I wonder if I can pester for you a code review 🙏

valerian-roche
valerian-roche previously approved these changes Oct 27, 2025

func (cache *LinearCache) Fetch(context.Context, *Request) (Response, error) {
return nil, errors.New("not implemented")
return nil, errors.New("Fetch is not implemented by LinearCache")
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: most linters will trigger on capitalized error messages. Not sure if this goes through here as matching the method name though

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah not sure, how this slipped. My IDE also missed it

Signed-off-by: sotiris <[email protected]>
@valerian-roche valerian-roche merged commit a96510d into envoyproxy:main Oct 28, 2025
5 checks passed
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