jerryshao commented on code in PR #12960:
URL: https://github.com/apache/gravitino/pull/12960#discussion_r3966174016
##########
mcp-server/mcp_server/core/context.py:
##########
@@ -147,38 +169,64 @@ def _service_auth(setting: Setting):
class GravitinoContext:
def __init__(self, setting: Setting):
+ # Enforced here (not only in do_main()) so any path that constructs a
+ # GravitinoContext directly - not just the CLI entrypoint - fails
+ # fast on an invalid Setting, matching the pre-per-request-metalake
+ # behavior where a bad Setting couldn't be constructed at all.
+ setting.validate_metalake()
+ setting.validate_oauth()
self._setting = setting
- self._default_client = RESTClientFactory.create_rest_client(
- setting.metalake,
- setting.gravitino_uri,
- startup_authorization(setting),
- auth=_service_auth(setting),
+ # Eagerly built only when a startup default is configured, so the
+ # common single-metalake deployment pays no extra cost. Left unset
+ # (None) when metalake resolution must come from a per-request header
+ # on every call (HTTP transport with no --metalake default).
+ self._default_client = (
+ RESTClientFactory.create_rest_client(
+ setting.metalake,
+ setting.gravitino_uri,
+ startup_authorization(setting),
+ auth=_service_auth(setting),
+ )
+ if setting.metalake
+ else None
)
- # LRU cache of per-principal clients keyed by the raw Authorization
header.
- # Safe without locking: rest_client() runs on the single asyncio event
- # loop and never awaits between lookup and insert.
- self._clients_by_auth: "OrderedDict[str, object]" = OrderedDict()
+ # LRU cache of per-principal clients keyed by (Authorization header,
+ # metalake). Safe without locking: rest_client() runs on the single
+ # asyncio event loop and never awaits between lookup and insert.
+ self._clients_by_auth: "OrderedDict[tuple, object]" = OrderedDict()
Review Comment:
Tightened to `OrderedDict[tuple[str, str], object]`.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
mcp-server/mcp_server/core/audit.py:
##########
@@ -74,13 +75,19 @@ def emit(
authorization denial being the common case), not only
authorization failures; inspect error_type to disambiguate.
error_type: Exception class name when outcome is "deny", empty
otherwise.
+ metalake: Metalake the call named, empty when it used the server's
+ configured default. Recorded because one server can now
+ serve several metalakes, so "which tenant did this touch"
+ is no longer answerable from the server config alone.
Review Comment:
Docstring corrected: the metalake is the resolved value, so a call relying
on the default records that default; it is empty only for tools that are not
metalake-scoped, such as the metalake listing.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
mcp-server/mcp_server/tools/statistic.py:
##########
@@ -36,7 +35,6 @@ async def list_statistics_for_metadata(
Args:
ctx (Context): The request context.
- metalake_name (str): The name of the metalake.
metadata_type (str): The type of metadata (e.g., table, column).
For
more, please refer to too 'metadata_type_to_fullname_formats'
Review Comment:
Fixed.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
