You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "MonkeyCanCode (via GitHub)" <gi...@apache.org> on 2025/12/28 03:05:45 UTC
[PR] feat (MCP): Enforce mypy for MCP [polaris-tools]
MonkeyCanCode opened a new pull request, #117:
URL: https://github.com/apache/polaris-tools/pull/117
Similar to https://github.com/apache/polaris/pull/3305, enforce mypy for MCP code base.
Here is the before the fix:
```
➜ mcp-server git:(mcp_mypy) ✗ uv run pre-commit run --all-files
[INFO] Installing environment for https://github.com/pre-commit/mirrors-mypy.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
debug statements (python)................................................Passed
ruff check...............................................................Passed
ruff format..............................................................Passed
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1
mcp-server/polaris_mcp/authorization.py:76: error: Function is missing a type annotation [no-untyped-def]
mcp-server/polaris_mcp/authorization.py:86: error: Value of type "tuple[str, float] | None" is not indexable [index]
mcp-server/polaris_mcp/authorization.py:118: error: Incompatible return value type (got "dict[str, str | None]", expected "dict[str, str] | None") [return-value]
mcp-server/polaris_mcp/authorization.py:123: error: Incompatible return value type (got "dict[str, str | None]", expected "dict[str, str] | None") [return-value]
mcp-server/polaris_mcp/tools/policy.py:152: error: Argument 3 to "_handle_list" of "PolarisPolicyTool" has incompatible type "str | None"; expected "str" [arg-type]
mcp-server/polaris_mcp/tools/policy.py:155: error: Argument 4 to "_handle_get" of "PolarisPolicyTool" has incompatible type "str | None"; expected "str" [arg-type]
mcp-server/polaris_mcp/tools/policy.py:158: error: Argument 4 to "_handle_create" of "PolarisPolicyTool" has incompatible type "str | None"; expected "str" [arg-type]
mcp-server/polaris_mcp/tools/policy.py:161: error: Argument 4 to "_handle_update" of "PolarisPolicyTool" has incompatible type "str | None"; expected "str" [arg-type]
mcp-server/polaris_mcp/tools/policy.py:164: error: Argument 4 to "_handle_delete" of "PolarisPolicyTool" has incompatible type "str | None"; expected "str" [arg-type]
mcp-server/polaris_mcp/tools/policy.py:167: error: Argument 4 to "_handle_attach" of "PolarisPolicyTool" has incompatible type "str | None"; expected "str" [arg-type]
mcp-server/polaris_mcp/tools/policy.py:170: error: Argument 4 to "_handle_detach" of "PolarisPolicyTool" has incompatible type "str | None"; expected "str" [arg-type]
mcp-server/polaris_mcp/server.py:418: error: Incompatible types in assignment (expression has type "dict[str, Any]", target has type "bool") [assignment]
mcp-server/int_test/client.py:118: error: Function is missing a type annotation [no-untyped-def]
mcp-server/int_test/client.py:187: error: Function is missing a return type annotation [no-untyped-def]
mcp-server/int_test/client.py:187: note: Use "-> None" if function does not return a value
mcp-server/tests/test_table_tool.py:108: error: Unsupported target for indexed assignment ("Collection[str]") [index]
mcp-server/tests/test_server.py:90: error: Argument 1 to "_normalize_namespace" has incompatible type "tuple[str, int]"; expected "str | Sequence[str]" [arg-type]
mcp-server/tests/test_rest_tool.py:108: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:109: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:110: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:111: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:112: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:113: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:114: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:116: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:158: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:159: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:160: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:161: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_rest_tool.py:162: error: Value of type "dict[str, Any] | None" is not indexable [index]
mcp-server/tests/test_config.py:67: error: Function is missing a type annotation for one or more arguments [no-untyped-def]
Found 30 errors in 8 files (checked 25 source files)
```
And with this PR:
```
➜ mcp-server git:(mcp_mypy) uv run pre-commit run --all-files
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
debug statements (python)................................................Passed
ruff check...............................................................Passed
ruff format..............................................................Passed
mypy.....................................................................Passed
```
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat (MCP): Enforce mypy for MCP [polaris-tools]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #117:
URL: https://github.com/apache/polaris-tools/pull/117#discussion_r2651941594
##########
mcp-server/polaris_mcp/tools/policy.py:
##########
@@ -147,31 +147,26 @@ def call(self, arguments: Any) -> ToolExecutionResult:
if isinstance(realm, str) and realm.strip():
delegate_args["realm"] = realm
- if normalized == "list":
- self._require_namespace(namespace, "list")
- self._handle_list(delegate_args, catalog, namespace)
- elif normalized == "get":
- self._require_namespace(namespace, "get")
- self._handle_get(arguments, delegate_args, catalog, namespace)
- elif normalized == "create":
- self._require_namespace(namespace, "create")
- self._handle_create(arguments, delegate_args, catalog, namespace)
- elif normalized == "update":
- self._require_namespace(namespace, "update")
- self._handle_update(arguments, delegate_args, catalog, namespace)
- elif normalized == "delete":
- self._require_namespace(namespace, "delete")
- self._handle_delete(arguments, delegate_args, catalog, namespace)
- elif normalized == "attach":
- self._require_namespace(namespace, "attach")
- self._handle_attach(arguments, delegate_args, catalog, namespace)
- elif normalized == "detach":
- self._require_namespace(namespace, "detach")
- self._handle_detach(arguments, delegate_args, catalog, namespace)
- elif normalized == "applicable":
+ if normalized == "applicable":
self._handle_applicable(delegate_args, catalog)
- else: # pragma: no cover
- raise ValueError(f"Unsupported operation: {operation}")
+ else:
+ str_namespace = self._require_namespace(namespace, normalized)
+ if normalized == "list":
+ self._handle_list(delegate_args, catalog, str_namespace)
+ elif normalized == "get":
+ self._handle_get(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "create":
+ self._handle_create(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "update":
+ self._handle_update(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "delete":
+ self._handle_delete(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "attach":
+ self._handle_attach(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "detach":
+ self._handle_detach(arguments, delegate_args, catalog, str_namespace)
+ else: # pragma: no cover
+ raise ValueError(f"Unsupported operation: {operation}")
Review Comment:
curious why we need this change
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat (MCP): Enforce mypy for MCP [polaris-tools]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on PR #117:
URL: https://github.com/apache/polaris-tools/pull/117#issuecomment-3697828680
> Thanks a lot for working on it, @MonkeyCanCode ! LGTM overall. Left one questions.
Thanks for the quick review. Responded on the comment.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat (MCP): Enforce mypy for MCP [polaris-tools]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #117:
URL: https://github.com/apache/polaris-tools/pull/117#discussion_r2651957403
##########
mcp-server/polaris_mcp/tools/policy.py:
##########
@@ -147,31 +147,26 @@ def call(self, arguments: Any) -> ToolExecutionResult:
if isinstance(realm, str) and realm.strip():
delegate_args["realm"] = realm
- if normalized == "list":
- self._require_namespace(namespace, "list")
- self._handle_list(delegate_args, catalog, namespace)
- elif normalized == "get":
- self._require_namespace(namespace, "get")
- self._handle_get(arguments, delegate_args, catalog, namespace)
- elif normalized == "create":
- self._require_namespace(namespace, "create")
- self._handle_create(arguments, delegate_args, catalog, namespace)
- elif normalized == "update":
- self._require_namespace(namespace, "update")
- self._handle_update(arguments, delegate_args, catalog, namespace)
- elif normalized == "delete":
- self._require_namespace(namespace, "delete")
- self._handle_delete(arguments, delegate_args, catalog, namespace)
- elif normalized == "attach":
- self._require_namespace(namespace, "attach")
- self._handle_attach(arguments, delegate_args, catalog, namespace)
- elif normalized == "detach":
- self._require_namespace(namespace, "detach")
- self._handle_detach(arguments, delegate_args, catalog, namespace)
- elif normalized == "applicable":
+ if normalized == "applicable":
self._handle_applicable(delegate_args, catalog)
- else: # pragma: no cover
- raise ValueError(f"Unsupported operation: {operation}")
+ else:
+ str_namespace = self._require_namespace(namespace, normalized)
+ if normalized == "list":
+ self._handle_list(delegate_args, catalog, str_namespace)
+ elif normalized == "get":
+ self._handle_get(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "create":
+ self._handle_create(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "update":
+ self._handle_update(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "delete":
+ self._handle_delete(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "attach":
+ self._handle_attach(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "detach":
+ self._handle_detach(arguments, delegate_args, catalog, str_namespace)
+ else: # pragma: no cover
+ raise ValueError(f"Unsupported operation: {operation}")
Review Comment:
So earlier we need to call `self._require_namespace(namespace, "list")` per normalized operation. and all of them use `namespace` as 3rd argument except `applicable`. The small nit here is to have the simplest form (the one with 2 arguments) to be on the top and everything else in the else clause for readability. As `namespace` is not relevant for `applicable` operation, thus I moved this to the top as it is the simplest form of function call.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat (MCP): Enforce mypy for MCP [polaris-tools]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #117:
URL: https://github.com/apache/polaris-tools/pull/117#discussion_r2651941594
##########
mcp-server/polaris_mcp/tools/policy.py:
##########
@@ -147,31 +147,26 @@ def call(self, arguments: Any) -> ToolExecutionResult:
if isinstance(realm, str) and realm.strip():
delegate_args["realm"] = realm
- if normalized == "list":
- self._require_namespace(namespace, "list")
- self._handle_list(delegate_args, catalog, namespace)
- elif normalized == "get":
- self._require_namespace(namespace, "get")
- self._handle_get(arguments, delegate_args, catalog, namespace)
- elif normalized == "create":
- self._require_namespace(namespace, "create")
- self._handle_create(arguments, delegate_args, catalog, namespace)
- elif normalized == "update":
- self._require_namespace(namespace, "update")
- self._handle_update(arguments, delegate_args, catalog, namespace)
- elif normalized == "delete":
- self._require_namespace(namespace, "delete")
- self._handle_delete(arguments, delegate_args, catalog, namespace)
- elif normalized == "attach":
- self._require_namespace(namespace, "attach")
- self._handle_attach(arguments, delegate_args, catalog, namespace)
- elif normalized == "detach":
- self._require_namespace(namespace, "detach")
- self._handle_detach(arguments, delegate_args, catalog, namespace)
- elif normalized == "applicable":
+ if normalized == "applicable":
self._handle_applicable(delegate_args, catalog)
- else: # pragma: no cover
- raise ValueError(f"Unsupported operation: {operation}")
+ else:
+ str_namespace = self._require_namespace(namespace, normalized)
+ if normalized == "list":
+ self._handle_list(delegate_args, catalog, str_namespace)
+ elif normalized == "get":
+ self._handle_get(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "create":
+ self._handle_create(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "update":
+ self._handle_update(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "delete":
+ self._handle_delete(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "attach":
+ self._handle_attach(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "detach":
+ self._handle_detach(arguments, delegate_args, catalog, str_namespace)
+ else: # pragma: no cover
+ raise ValueError(f"Unsupported operation: {operation}")
Review Comment:
curious why we need this change from line 150 to line 169
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat (MCP): Enforce mypy for MCP [polaris-tools]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #117:
URL: https://github.com/apache/polaris-tools/pull/117#discussion_r2651960991
##########
mcp-server/polaris_mcp/tools/policy.py:
##########
@@ -147,31 +147,26 @@ def call(self, arguments: Any) -> ToolExecutionResult:
if isinstance(realm, str) and realm.strip():
delegate_args["realm"] = realm
- if normalized == "list":
- self._require_namespace(namespace, "list")
- self._handle_list(delegate_args, catalog, namespace)
- elif normalized == "get":
- self._require_namespace(namespace, "get")
- self._handle_get(arguments, delegate_args, catalog, namespace)
- elif normalized == "create":
- self._require_namespace(namespace, "create")
- self._handle_create(arguments, delegate_args, catalog, namespace)
- elif normalized == "update":
- self._require_namespace(namespace, "update")
- self._handle_update(arguments, delegate_args, catalog, namespace)
- elif normalized == "delete":
- self._require_namespace(namespace, "delete")
- self._handle_delete(arguments, delegate_args, catalog, namespace)
- elif normalized == "attach":
- self._require_namespace(namespace, "attach")
- self._handle_attach(arguments, delegate_args, catalog, namespace)
- elif normalized == "detach":
- self._require_namespace(namespace, "detach")
- self._handle_detach(arguments, delegate_args, catalog, namespace)
- elif normalized == "applicable":
+ if normalized == "applicable":
self._handle_applicable(delegate_args, catalog)
- else: # pragma: no cover
- raise ValueError(f"Unsupported operation: {operation}")
+ else:
+ str_namespace = self._require_namespace(namespace, normalized)
+ if normalized == "list":
+ self._handle_list(delegate_args, catalog, str_namespace)
+ elif normalized == "get":
+ self._handle_get(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "create":
+ self._handle_create(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "update":
+ self._handle_update(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "delete":
+ self._handle_delete(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "attach":
+ self._handle_attach(arguments, delegate_args, catalog, str_namespace)
+ elif normalized == "detach":
+ self._handle_detach(arguments, delegate_args, catalog, str_namespace)
+ else: # pragma: no cover
+ raise ValueError(f"Unsupported operation: {operation}")
Review Comment:
Thanks for the explanation. Always good to have a less scoped variable!
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat (MCP): Enforce mypy for MCP [polaris-tools]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode merged PR #117:
URL: https://github.com/apache/polaris-tools/pull/117
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org