feat: migrate Thin SDK to sidecar control session - #1
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the Thin SDK’s Sidecar bootstrap from the legacy server-streaming OpenSession to the bidirectional OpenControlSession, and adds a local-service registration lifecycle that is replayed after reconnects.
Changes:
- Switch Sidecar session establishment to
OpenControlSessionand handle first-frame listener snapshot plus subsequent status events. - Add local service register/unregister APIs, desired-state tracking, and reconnect replay.
- Update README usage guidance and extend the sidecar session test to cover registration + replay + unregistration.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/sidecar_session_test.cc | Updates the test server + client flow for OpenControlSession, and validates registration replay/unregistration behavior. |
| src/sidecar_session.cc | Implements bidirectional control stream, desired registration replay, and local service status handling. |
| include/pole/client/sidecar_session.h | Exposes new local-service registration/status APIs and adds internal state for replay/status tracking. |
| README.md | Documents the new OpenControlSession behavior and the local service registration API. |
| context-kg/tasks/todo.md | Adds a migration task checklist and verification notes. |
Suppressed comments (1)
src/sidecar_session.cc:266
SendUnregistrationignores the return value ofWrite(). If the control stream is already closed, the unregister request is silently dropped and may not trigger a reconnect, so the desired state change can be delayed. Consider cancelling the active context when a write fails to force a reconnect.
::pole::sidecar::v1::ClientEvent event;
event.mutable_unregister_local_service()->set_registration_id(registration_id);
active_stream_->writer->Write(event);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| wire_registration->set_service(registration.service); | ||
| wire_registration->set_protocol(ToWireProtocol(registration.protocol)); | ||
| wire_registration->set_local_port(registration.local_port); | ||
| active_stream_->writer->Write(event); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
验证