feat: Update gRPC definitions for Limit Enforcer #12
Conversation
- Update service.proto with node_id, panel_api_url, limit params - Regenerate service_pb2.py and service_grpc.py - Update abstract_node.py with limit enforcer params - Update grpclib.py and rest.py with new start method signature
WalkthroughConfiguration parameters for limit enforcement (node_id, panel_api_url, limit_check_interval, limit_refresh_interval) are added to the PasarGuardNode.start method signature across the abstract interface and concrete implementations, with corresponding protobuf message schema updates to transmit these settings. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@PasarGuardNodeBridge/common/service.proto`:
- Around line 26-31: The proto comments saying "default 30/60" are misleading
because proto3 scalars decode to zero; update handling for the fields
limit_check_interval and limit_refresh_interval so zero values don't become
0-second intervals: either change the proto to use optional or wrapper types
(e.g., google.protobuf.Int32Value) for limit_check_interval and
limit_refresh_interval so presence can be detected, or keep scalars but add
normalization logic in the node/panel code that reads node_id, panel_api_url,
limit_check_interval, and limit_refresh_interval and replaces 0 with the
intended defaults (30 and 60) before using them.
🧹 Nitpick comments (2)
PasarGuardNodeBridge/rest.py (1)
115-149: Avoid mutable default forexclude_inbounds.Using a list literal as a default argument can leak state across calls. Prefer
Noneand initialize inside the method.♻️ Proposed fix
- async def start( + async def start( self, config: str, backend_type: service.BackendType, users: list[service.User], keep_alive: int = 0, - exclude_inbounds: list[str] = [], + exclude_inbounds: list[str] | None = None, timeout: int | None = None, # Limit enforcer configuration (optional - sent to node for real-time enforcement) node_id: int = 0, panel_api_url: str = "", limit_check_interval: int = 30, limit_refresh_interval: int = 60, ): """Start the node with proper task management""" timeout = timeout or self._default_timeout + exclude_inbounds = exclude_inbounds or []PasarGuardNodeBridge/grpclib.py (1)
100-130: Avoid mutable default forexclude_inbounds.Use
None+ initialization to avoid shared state across calls.♻️ Proposed fix
- async def start( + async def start( self, config: str, backend_type: service.BackendType, users: list[service.User], keep_alive: int = 0, - exclude_inbounds: list[str] = [], + exclude_inbounds: list[str] | None = None, timeout: int | None = None, # Limit enforcer configuration (optional - sent to node for real-time enforcement) node_id: int = 0, panel_api_url: str = "", limit_check_interval: int = 30, limit_refresh_interval: int = 60, ) -> service.BaseInfoResponse | None: """Start the node with proper task management""" timeout = timeout or self._default_timeout + exclude_inbounds = exclude_inbounds or []
|
|
||
| // Limit enforcer configuration (sent from panel) | ||
| int32 node_id = 6; | ||
| string panel_api_url = 7; | ||
| int32 limit_check_interval = 8; // seconds, default 30 | ||
| int32 limit_refresh_interval = 9; // seconds, default 60 |
There was a problem hiding this comment.
Proto3 “default” comments are not enforced.
In proto3, scalar fields default to zero on decode; the “default 30/60” comments won’t apply unless the server/client normalizes. This can lead to 0-second intervals if another client omits the fields.
Consider either:
- Normalizing
0to30/60in the node/panel logic, or - Using
optional/wrapper types to detect presence.
💡 Minimal clarification (comment-only) if you keep current schema
- int32 limit_check_interval = 8; // seconds, default 30
- int32 limit_refresh_interval = 9; // seconds, default 60
+ int32 limit_check_interval = 8; // seconds; 0 means "use default" (e.g., 30)
+ int32 limit_refresh_interval = 9; // seconds; 0 means "use default" (e.g., 60)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Limit enforcer configuration (sent from panel) | |
| int32 node_id = 6; | |
| string panel_api_url = 7; | |
| int32 limit_check_interval = 8; // seconds, default 30 | |
| int32 limit_refresh_interval = 9; // seconds, default 60 | |
| // Limit enforcer configuration (sent from panel) | |
| int32 node_id = 6; | |
| string panel_api_url = 7; | |
| int32 limit_check_interval = 8; // seconds; 0 means "use default" (e.g., 30) | |
| int32 limit_refresh_interval = 9; // seconds; 0 means "use default" (e.g., 60) |
🤖 Prompt for AI Agents
In `@PasarGuardNodeBridge/common/service.proto` around lines 26 - 31, The proto
comments saying "default 30/60" are misleading because proto3 scalars decode to
zero; update handling for the fields limit_check_interval and
limit_refresh_interval so zero values don't become 0-second intervals: either
change the proto to use optional or wrapper types (e.g.,
google.protobuf.Int32Value) for limit_check_interval and limit_refresh_interval
so presence can be detected, or keep scalars but add normalization logic in the
node/panel code that reads node_id, panel_api_url, limit_check_interval, and
limit_refresh_interval and replaces 0 with the intended defaults (30 and 60)
before using them.
1 participant
Summary
Updates the gRPC service definitions to support per-node limit configuration.
Changes
service.protowith new fields for limit enforcement params.service_pb2.pyandservice_grpc.py.Related PRs
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.