Skip to content

fix: implement RegistryServer.Proto gRPC method#6552

Open
nquinn408 wants to merge 1 commit into
feast-dev:masterfrom
nquinn408:fix-remote
Open

fix: implement RegistryServer.Proto gRPC method#6552
nquinn408 wants to merge 1 commit into
feast-dev:masterfrom
nquinn408:fix-remote

Conversation

@nquinn408

Copy link
Copy Markdown
Contributor

The remote registry client (RemoteRegistry.proto) calls stub.Proto(Empty()) to fetch the full registry, but the RegistryServer servicer never implemented the Proto RPC, so remote clients received UNIMPLEMENTED. Return proxied_registry.proto().

The remote registry client (RemoteRegistry.proto) calls stub.Proto(Empty())
to fetch the full registry, but the RegistryServer servicer never implemented
the Proto RPC, so remote clients received UNIMPLEMENTED. Return
proxied_registry.proto().

Signed-off-by: Nick Quinn <nicholas_quinn@apple.com>
@nquinn408 nquinn408 self-assigned this Jun 25, 2026
@nquinn408 nquinn408 requested a review from a team as a code owner June 25, 2026 14:43
self.proxied_registry = registry

def Proto(self, request: Empty, context) -> RegistryProto:
return self.proxied_registry.proto()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@nquinn408 it was intentionally removed, so that it won't return all information to the users not having access. Probably proper rbac needs to be added if we need it for any purpose.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I understand @ntkathole but then it breaks RemoteRegistry.proto, so I don't know what the solution is. I guess a different implementation for RemoteRegistry.proto?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

def proto(self) -> RegistryProto:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can construct the response by calling each typed List* method (have RBAC checks) rather than forwarding the raw proto().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there an open issue to address? I will give it a shot.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

created #6558

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.

3 participants