Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the Python net_sync_manager UDP discovery shutdown logic and adds an integrated “discover → apply endpoint → connect” flow to match the behavior expected in Issue #365.
Changes:
- Extracted discovery shutdown into
_stop_discovery_internal()and updatedstop_discovery()to clear the auto-restart port before stopping sockets/thread. - Enhanced
_discovery_loop()to apply discovered server endpoint, stop discovery, and auto-start the client connection when appropriate.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add self-join guard in _stop_discovery_internal() to prevent RuntimeError when called from the discovery thread - Only update server/dealer_port/sub_port when not already connected - Retry discovery on start() failure instead of leaving client dead - Add tests for auto-connect, self-join safety, and failure recovery Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
This pull request refactors the discovery stopping logic and introduces an auto-connect feature after a server is discovered. The main improvements are better separation of concerns in stopping discovery and automatic connection initiation upon successful discovery.
Discovery logic improvements:
_stop_discovery_internal, improving code reuse and clarity. Now,stop_discovery()calls this internal method after clearing the discovery port, ensuring that the auto-restart mechanism is handled correctly.Auto-connect enhancement:
_discovery_loopmethod: after a server is discovered, the client updates its internal state, stops the discovery process, and automatically attempts to start a connection if not already running. Errors during this process are logged for easier debugging.Close #365