-
Notifications
You must be signed in to change notification settings - Fork 42
Added gRPC gnmi protocol to UTCP #82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from 22 commits
d28dc58
018806c
0f2af7e
d28c0af
7ba8b3c
908cd40
74a11e2
03a4b9f
8443cda
0150a3b
6e2c671
9cea90f
7016987
718b668
ca252e5
45793cf
662d07d
3aed349
dca4d26
4a2aea4
21cbab7
700ec92
70ff230
9bbb819
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| [build-system] | ||
| requires = ["setuptools>=61.0"] | ||
| build-backend = "setuptools.build_meta" | ||
|
|
||
| [project] | ||
| name = "utcp-gnmi" | ||
| version = "1.0.0" | ||
| authors = [ | ||
| { name = "UTCP Contributors" }, | ||
| ] | ||
| description = "UTCP gNMI communication protocol plugin over gRPC" | ||
| readme = "README.md" | ||
| requires-python = ">=3.10" | ||
| dependencies = [ | ||
| "pydantic>=2.0", | ||
| "protobuf>=4.21", | ||
| "grpcio>=1.60", | ||
| "utcp>=1.0", | ||
| "aiohttp>=3.8" | ||
| ] | ||
| license = "MPL-2.0" | ||
|
|
||
| [project.optional-dependencies] | ||
| dev = [ | ||
| "build", | ||
| "pytest", | ||
| "pytest-asyncio", | ||
| "pytest-cov", | ||
| ] | ||
|
|
||
| [project.entry-points."utcp.plugins"] | ||
| gnmi = "utcp_gnmi:register" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| from utcp.plugins.discovery import register_communication_protocol, register_call_template | ||
| from utcp_gnmi.gnmi_communication_protocol import GnmiCommunicationProtocol | ||
| from utcp_gnmi.gnmi_call_template import GnmiCallTemplateSerializer | ||
|
|
||
| def register(): | ||
| register_communication_protocol("gnmi", GnmiCommunicationProtocol()) | ||
| register_call_template("gnmi", GnmiCallTemplateSerializer()) | ||
|
|
||
| __all__ = [ | ||
| "GnmiCommunicationProtocol", | ||
| "GnmiCallTemplateSerializer", | ||
| ] |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,27 @@ | ||||
| from typing import Optional, Dict, List, Literal | ||||
| from pydantic import Field | ||||
|
||||
| from pydantic import Field |
h3xxit marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,245 @@ | ||||||||||||||||||||||||||||||||
| import importlib | ||||||||||||||||||||||||||||||||
| from typing import Dict, Any, List, Optional, AsyncGenerator | ||||||||||||||||||||||||||||||||
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| from utcp.interfaces.communication_protocol import CommunicationProtocol | ||||||||||||||||||||||||||||||||
| from utcp.data.call_template import CallTemplate | ||||||||||||||||||||||||||||||||
| from utcp.data.tool import Tool, JsonSchema | ||||||||||||||||||||||||||||||||
| from utcp.data.utcp_manual import UtcpManual | ||||||||||||||||||||||||||||||||
| from utcp.data.register_manual_response import RegisterManualResult | ||||||||||||||||||||||||||||||||
| from utcp_gnmi.gnmi_call_template import GnmiCallTemplate | ||||||||||||||||||||||||||||||||
| from utcp.data.auth_implementations.api_key_auth import ApiKeyAuth | ||||||||||||||||||||||||||||||||
| from utcp.data.auth_implementations.basic_auth import BasicAuth | ||||||||||||||||||||||||||||||||
| from utcp.data.auth_implementations.oauth2_auth import OAuth2Auth | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| class GnmiCommunicationProtocol(CommunicationProtocol): | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| class GnmiCommunicationProtocol(CommunicationProtocol): | |
| class GnmiCommunicationProtocol(CommunicationProtocol): | |
| def __init__(self): | |
| super().__init__() | |
| self._oauth_tokens = {} |
Outdated
Copilot
AI
Dec 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The empty pass statement in the TLS branch serves no purpose and makes the code harder to read. Consider removing it or adding a comment explaining why TLS is always allowed.
| if manual_call_template.use_tls: | |
| pass | |
| else: | |
| if not manual_call_template.use_tls: |
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The manual version should come from the manual endpoint. You can be opinionated on how the UtcpManual should be transfered accross the network, and ideally keept the transfered object as close to the original UtcpManual as possible, so that the user on the other side can basically just expose an endpoint that returns this UtcpManual json/object
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Dec 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The module import and stub discovery logic (lines 95-114) is duplicated in both call_tool and call_tool_streaming (lines 168-186). This duplication creates a maintenance burden. Consider extracting this into a private helper method like _create_grpc_stub to eliminate the duplication.
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Dec 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getattr(msg_mod, "SubscriptionList".upper(), None) is incorrect and will always evaluate to None. It attempts to get an attribute named "SUBSCRIPTIONLIST" (all uppercase string literal) instead of the subscription mode enum value. This should likely retrieve the subscription mode from the message module based on the mode variable, such as getattr(msg_mod, f"SubscriptionList.Mode.{mode}", 0) or similar, depending on the protobuf structure.
| sub_list.mode = getattr(msg_mod, "SubscriptionList".upper(), None) or 0 | |
| # Set the mode enum value correctly | |
| try: | |
| mode_enum = getattr(getattr(msg_mod, "SubscriptionList"), "Mode") | |
| sub_list.mode = getattr(mode_enum, mode, 0) | |
| except AttributeError: | |
| sub_list.mode = 0 |
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Dec 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message "Subscribe is streaming; use call_tool_streaming" is confusing in the context of the subscribe operation. The subscribe request is being built (lines 142-154) but then immediately raises an error. Either the subscribe case should be removed from call_tool entirely, or this error message should clarify that the operation was attempted through the wrong method. Consider removing this case from call_tool and letting the "Unsupported gNMI operation" error on line 157 handle it instead.
| elif op == "subscribe": | |
| req = getattr(msg_mod, "SubscribeRequest")() | |
| sub_list = getattr(msg_mod, "SubscriptionList")() | |
| mode = tool_args.get("mode", "stream").upper() | |
| sub_list.mode = getattr(msg_mod, "SubscriptionList".upper(), None) or 0 | |
| paths = tool_args.get("paths", []) | |
| for p in paths: | |
| path_msg = getattr(msg_mod, "Path")() | |
| for elem in [e for e in p.strip("/").split("/") if e]: | |
| pe = getattr(msg_mod, "PathElem")(name=elem) | |
| path_msg.elem.append(pe) | |
| sub = getattr(msg_mod, "Subscription")(path=path_msg) | |
| sub_list.subscription.append(sub) | |
| req.subscribe.CopyFrom(sub_list) | |
| raise ValueError("Subscribe is streaming; use call_tool_streaming") |
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Dec 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The gRPC channel created on lines 174-178 is never closed. gRPC channels should be properly closed after use to avoid resource leaks. Consider wrapping the channel usage in a try-finally block or using async context manager pattern to ensure the channel is closed even if the streaming operation is interrupted.
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
h3xxit marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Dec 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OAuth2 token fetching does not cache tokens like other protocol implementations do (e.g., HTTP, GraphQL). This will result in fetching a new token on every call, which is inefficient and may hit rate limits. The implementation should store tokens in an instance variable (e.g., self._oauth_tokens) and check for cached tokens before making a request, similar to the GraphQL implementation at lines 56-58.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import sys | ||
| from pathlib import Path | ||
| import pytest | ||
|
|
||
| plugin_src = Path(__file__).parent.parent / "src" | ||
| sys.path.insert(0, str(plugin_src)) | ||
|
|
||
| core_src = Path(__file__).parent.parent.parent.parent.parent / "core" / "src" | ||
| sys.path.insert(0, str(core_src)) | ||
|
|
||
| from utcp.utcp_client import UtcpClient | ||
| from utcp_gnmi import register | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_register_manual_and_tools(): | ||
| register() | ||
| client = await UtcpClient.create(config={ | ||
| "manual_call_templates": [ | ||
| { | ||
| "name": "routerA", | ||
| "call_template_type": "gnmi", | ||
| "target": "localhost:50051", | ||
| "use_tls": False, | ||
| "operation": "get" | ||
| } | ||
| ] | ||
| }) | ||
| tools = await client.config.tool_repository.get_tools() | ||
| names = [t.name for t in tools] | ||
| assert any(n.startswith("routerA.") for n in names) | ||
| assert any(n.endswith("subscribe") for n in names) | ||
|
Comment on lines
+14
to
+31
|
||
|
|
||
| def test_serializer_roundtrip(): | ||
| from utcp_gnmi.gnmi_call_template import GnmiCallTemplateSerializer | ||
| serializer = GnmiCallTemplateSerializer() | ||
| data = { | ||
| "name": "routerB", | ||
| "call_template_type": "gnmi", | ||
| "target": "localhost:50051", | ||
| "use_tls": False, | ||
| "metadata": {"authorization": "Bearer token"}, | ||
| "metadata_fields": ["tenant-id"], | ||
| "operation": "set", | ||
| "stub_module": "gnmi_pb2_grpc", | ||
| "message_module": "gnmi_pb2" | ||
| } | ||
| obj = serializer.validate_dict(data) | ||
| out = serializer.to_dict(obj) | ||
| assert out["call_template_type"] == "gnmi" | ||
| assert out["operation"] == "set" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pyproject.toml references a README.md file that does not exist in the gnmi plugin directory. This will cause packaging issues. Either create a README.md file with appropriate documentation (similar to the GraphQL plugin's README) or remove the readme field from the project configuration.