ClustersClientsPool Middleware #1627
Conversation
Add text filtering
| if err != nil { | ||
| return nil, fmt.Errorf("converting items: %w", err) | ||
| //TODO: handle failures and parallelize | ||
| for _, c := range clientsPool.Clients() { |
There was a problem hiding this comment.
Can we add a clusterName field to the response payload? The UI will need that to show the name of the cluster in the table.
I think it will need to be an argument to the types.KustomizationToProto function.
There was a problem hiding this comment.
Given this work is focused on a single cluster, I'll leave this to later because adding these names is nontrivial since we'll have to change the response format and update the UI to comply with it.
luizbafilho
force-pushed
the
lf/k8s-clients-pool
branch
from
9c05af5 to
6512b46
Compare
| return | ||
| } | ||
|
|
||
| clusters, err := clustersFetcher.Fetch(r.Context()) |
There was a problem hiding this comment.
we are punting the caching of the available clusters to another PR right?
tho i suppose the implementor of the interface can cache on that side. actually that is the better idea, nvm!
There was a problem hiding this comment.
yep, not handling any of that now.
Co-authored-by: Claudia <claudiaberesford@gmail.com>
| @@ -74,3 +76,23 @@ func makeGRPCServer(cfg *rest.Config, t *testing.T) pb.CoreClient { | |||
|
|
|||
| return pb.NewCoreClient(conn) | |||
| } | |||
|
|
|||
| func withClientsPoolInterceptor(config *rest.Config, user *auth.UserPrincipal) grpc.ServerOption { | |||
| return grpc.UnaryInterceptor(func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { | |||
There was a problem hiding this comment.
Are we sure this runs? I have run into issues before where the middlewares don't run with our current implementation: grpc-ecosystem/grpc-gateway#1043
There was a problem hiding this comment.
yes, if we comment that out the test breaks because the clients pool doesnt get injected
There was a problem hiding this comment.
What about when the server is actually running?
There was a problem hiding this comment.
I don't know, if you pay attention to that's for test only since we use HTTP middleware for it.
Closes: #1600
What changed?
Why?
How did you test it?
Release notes
Documentation Changes