Add BridgeGRpcClient #19546
Add BridgeGRpcClient #19546
Conversation
|
🟢 |
mregrock
force-pushed
the
bridge_rpc_call
branch
from
56bc6d7 to
d7a18bf
Compare
|
⚪ Test history | Ya make output | Test bloat
⚪
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
There was a problem hiding this comment.
Pull Request Overview
Introduces a new “bridge” service and client to manage per-pile cluster state via gRPC.
- Registers a
bridgemodule in build scripts and YaMake files. - Implements
TBridgeGRpcServiceand backend handlers (rpc_bridge.cpp) for Get/UpdateClusterState. - Adds
TBridgeClientin the C++ SDK along with protos (ydb_bridge.proto/ydb_bridge_v1.proto) and integrates the service into the runner and tracing.
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ydb/services/ya.make | Added bridge to services build recursion |
| ydb/services/bridge/ya.make | New build file for the bridge service |
| ydb/services/bridge/grpc_service.h | Declares TBridgeGRpcService |
| ydb/services/bridge/grpc_service.cpp | Implements service setup and incoming requests |
| ydb/public/sdk/cpp/src/client/bridge/ya.make | Build file for the C++ bridge client |
| ydb/public/sdk/cpp/src/client/bridge/bridge.cpp | Implements TBridgeClient methods |
| ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/bridge/bridge.h | Public header for TBridgeClient API |
| ydb/public/api/protos/ya.make | Includes ydb_bridge.proto |
| ydb/public/api/protos/draft/ydb_bridge.proto | Defines bridge‐related protobuf messages |
| ydb/public/api/grpc/draft/ydb_bridge_v1.proto | Defines BridgeService RPC methods |
| ydb/public/api/grpc/draft/ya.make | Includes ydb_bridge_v1.proto |
| ydb/library/services/services.proto | Adds BRIDGE enum to Kikimr services |
| ydb/core/jaeger_tracing/request_discriminator.h | Registers BRIDGE_GETCLUSTERSTATE / UPDATE types |
| ydb/core/jaeger_tracing/request_discriminator.cpp | Maps bridge RPC names to discriminator types |
| ydb/core/grpc_services/ya.make | Adds rpc_bridge.cpp to core gRPC services |
| ydb/core/grpc_services/service_bridge.h | Declares internal handlers DoGet/UpdateClusterState |
| ydb/core/grpc_services/rpc_bridge_base.h | Base template for bridge request actors |
| ydb/core/grpc_services/rpc_bridge.cpp | Implements bridge operation actor logic |
| ydb/core/driver_lib/run/ya.make | Pulls in the bridge service for the driver |
| ydb/core/driver_lib/run/run.cpp | Registers TBridgeGRpcService in the runner |
Comments suppressed due to low confidence (3)
ydb/services/bridge/grpc_service.h:27
- The member name
GRpcRequestProxyIdmixes uppercaseGand lowercaser. Rename toGrpcRequestProxyIdto match the commonGrpccasing elsewhere.
NActors::TActorId GRpcRequestProxyId;
ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/bridge/bridge.h:1
- The public API header lacks Doxygen or doc comments. Consider adding brief descriptions for
TBridgeClientand its methods to match the style of other SDK clients.
#pragma once
ydb/public/sdk/cpp/src/client/bridge/bridge.cpp:8
- No unit or integration tests accompany the new client code. Add tests for
UpdateClusterStateandGetClusterStateto verify serialization, error handling, and response extraction.
namespace NYdb::inline Dev::NBridge {
ydb/services/bridge/grpc_service.cpp
Outdated
| TBridgeGRpcService::TBridgeGRpcService(NActors::TActorSystem* actorSystem, TIntrusivePtr<NMonitoring::TDynamicCounters> counters, NActors::TActorId grpcRequestProxyId) \ | ||
| : ActorSystem(actorSystem) \ | ||
| , Counters(std::move(counters)) | ||
| , GRpcRequestProxyId(grpcRequestProxyId) |
There was a problem hiding this comment.
[nitpick] Avoid using backslashes for line continuation in C++ source files; reformat the constructor initializer list onto multiple lines without \ for better readability.
| TBridgeGRpcService::TBridgeGRpcService(NActors::TActorSystem* actorSystem, TIntrusivePtr<NMonitoring::TDynamicCounters> counters, NActors::TActorId grpcRequestProxyId) \ | |
| : ActorSystem(actorSystem) \ | |
| , Counters(std::move(counters)) | |
| , GRpcRequestProxyId(grpcRequestProxyId) | |
| TBridgeGRpcService::TBridgeGRpcService( | |
| NActors::TActorSystem* actorSystem, | |
| TIntrusivePtr<NMonitoring::TDynamicCounters> counters, | |
| NActors::TActorId grpcRequestProxyId) | |
| : ActorSystem(actorSystem), | |
| Counters(std::move(counters)), | |
| GRpcRequestProxyId(grpcRequestProxyId) |
Copilot uses AI. Check for mistakes.
mregrock
force-pushed
the
bridge_rpc_call
branch
from
d7a18bf to
77fbabe
Compare
mregrock
force-pushed
the
bridge_rpc_call
branch
from
77fbabe to
c22345d
Compare
mregrock
force-pushed
the
bridge_rpc_call
branch
from
c22345d to
04cab53
Compare
mregrock
force-pushed
the
bridge_rpc_call
branch
from
04cab53 to
507861e
Compare
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| public: | ||
| using TBase::TBase; | ||
|
|
||
| void Bootstrap(const TActorContext& ctx) { |
There was a problem hiding this comment.
You don't need TActorContext anymore. Not in new code.
| return false; | ||
| } | ||
|
|
||
| std::optional<ui32> primary, promoted; |
ydb/public/sdk/cpp/include/ydb-cpp-sdk/client/draft/ydb_bridge.h
Outdated
Show resolved
Hide resolved
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Changelog entry
Add BridgeGRpcClient
Changelog category
Description for reviewers
...