chore: Managing tenant in JSONRPC. #613
Conversation
If the tenant is not defined in the jsonrpc payload, we use the one from the request path. Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
Summary of ChangesHello @ehsavoie, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the application's multi-tenancy capabilities by introducing a robust mechanism to manage tenant identification within JSONRPC requests. It ensures that if a tenant is not explicitly provided in the JSONRPC payload, it is automatically derived from the incoming request's URL path. This change streamlines tenant context propagation, making the system more resilient and consistent in handling tenant-specific operations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request successfully implements the feature of managing tenant information in both JSONRPC and REST requests. The extractTenant methods correctly derive the tenant from the request path, and the JSONRPCUtils.parseRequestBody method intelligently applies this tenant information only if it's not already present in the JSONRPC payload. The added test cases in A2AServerRoutesTest.java provide good coverage for the tenant extraction logic, covering various path scenarios. The changes are well-contained and effectively address the stated objective.
|
I don't really get how multinenancy is supposed to work in A2A (even after reading a2aproject/A2A#1273) but the resolution of the tenant from the url path looks very suspicious. {
"url": "https://my.example.com/a2a",
"protocolBinding": "REST",
"tenant": "/project/123/agent/345"
} |
My understanding is that the agent should know how to remove the a2a part if the tenant is something it needs to handle |
Ok, the code I pointed out is in the reference bits which are a simple implementation of an agent. If another agent wants to support multitenancy with different approaches, it's up to them... |
If the tenant is not defined in the JSONRPC payload, we use the one from the request path.
Fixes #612 🦕