Extract a transport-agnostic network discovery backend#15028
Open
rtibblesbot wants to merge 7 commits into
Open
Extract a transport-agnostic network discovery backend#15028rtibblesbot wants to merge 7 commits into
rtibblesbot wants to merge 7 commits into
Conversation
github-actions
Bot
added
DEV: backend
rtibblesbot
marked this pull request as ready for review
July 17, 2026 20:52
rtibblesbot
force-pushed
the
issue-14996-cbedc0
branch
from
July 17, 2026 20:54
e0202eb to
de616e4
Compare
rtibbles
reviewed
Jul 17, 2026
| } | ||
|
|
||
|
|
||
| @register_hook(as_default=True) |
Member
There was a problem hiding this comment.
I think this could live in a kolibri_plugin.py inside discovery instead of going into the core.
Contributor
Author
There was a problem hiding this comment.
Good call — moved the registration into a new kolibri/core/discovery/kolibri_plugin.py, so it now lives alongside the transport it registers rather than in core. register_non_plugins loads it automatically since kolibri.core.discovery is an installed app. Done in 9fd6518.
rtibblesbot
force-pushed
the
issue-14996-cbedc0
branch
from
July 17, 2026 21:37
9fd6518 to
4a4903c
Compare
Contributor
Build Artifacts
Smoke test screenshot |
Split the zeroconf-welded KolibriBroadcast into a transport-agnostic NetworkDiscoveryBackend and a NetworkDiscoveryHook transport interface, so a platform can supply an alternative discovery transport (e.g. Android native NSD) and run discovery outside the server ProcessBus. Zeroconf becomes the default-registered transport; server behaviour is unchanged. - Define NetworkDiscoveryHook, a single-registration hook with register/update/unregister/start_listening/stop_listening. - Add NetworkDiscoveryBackend: broadcast-id lifecycle, the instance event bus, the other_instances dedup cache, and listener dispatch. It resolves its transport through the hook and is constructable/start-stoppable without a KolibriProcessBus. - Extract the zeroconf logic (Zeroconf/ServiceBrowser/ServiceInfo, interface and address monitoring, .local aliasing) into ZeroconfNetworkDiscovery, registered as the default transport via ZeroconfNetworkDiscoveryHook in kolibri/core/kolibri_plugin.py. - Move the LocalHostnameListener onto the transport's own bus. - Drive ZeroConfPlugin through NetworkDiscoveryBackend and remove KolibriBroadcast and its now-dead zeroconf-only event machinery. on_remove takes the service name (a removed service can no longer be queried) and resets the cached instance's broadcasting flag, so a peer leave then rejoin re-publishes an ADD. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NetworkDiscoveryBackend.update_broadcast emitted its broadcast-id log line through the root logger rather than the module-level logger, inconsistent with the rest of the module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The monitor thread starts on START but the backend isn't created until the later RUN transition (and is torn down on STOP), so a 5s tick can fire while there is no backend. Unconditionally dereferencing self.backend raised AttributeError, which BackgroundTask re-raises, permanently killing the monitor thread and stopping dynamic re-broadcast. The pre-split code guarded this via the addresses_changed property's broadcast-is-not-None check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The zeroconf transport's .local hostname events moved onto a new bus during the KolibriBroadcast split, but it was instantiated as a plain magicbus.base.Bus, which has no throws attribute. Bus.publish references self.throws in its listener error path, so any exception raised by a local-name listener (LocalHostnameListener -> sync_local_hostnames.enqueue) surfaced as a masking AttributeError instead of propagating. Restore the original KolibriBroadcastEvents behaviour with a LocalNameEvents bus subclass that sets throws = (Exception,), mirroring NetworkDiscoveryEvents. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The KolibriBroadcast test-class deletion dropped ~13 tests covering the .local hostname aliasing (bare/device-name aliases, multi-homed LAN address selection, outgoing-interface preference, conflict-skipping, rename and unregister handling). That behaviour was copied verbatim into ZeroconfNetworkDiscovery, not changed, so port the tests onto the transport test case rather than leaving the transport's local-name methods untested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extract a DiscoveryEventBus base carrying the throws configuration and an EventBusHost mixin providing add_listener, removing the duplicated copies from the instance and local-name buses and from both discovery hosts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the ZeroconfNetworkDiscoveryHook registration out of core's kolibri_plugin.py and into a dedicated kolibri_plugin.py inside the discovery app, where it lives alongside the transport it registers. register_non_plugins loads it automatically since kolibri.core.discovery is an installed app. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rtibblesbot
force-pushed
the
issue-14996-cbedc0
branch
from
July 17, 2026 22:11
4a4903c to
500f52e
Compare
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
2 participants
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.
Summary
Network discovery — mDNS broadcast plus peer listening — was welded to zeroconf inside
KolibriBroadcastand only ran inside the serverProcessBus, so no platform could supply an alternative transport or run discovery elsewhere. This extracts a transport-agnosticNetworkDiscoveryBackendand a single-registrationNetworkDiscoveryHooktransport interface, with the zeroconf implementation moved behind that hook and registered as the default. The backend owns the broadcast-id lifecycle, event bus, dedup cache, and listener dispatch, and can be constructed and started/stopped without aKolibriProcessBus. Server broadcast and discovery behaviour is unchanged.References
Fixes #14996.
Reviewer guidance
kolibri/core/discovery/utils/network/broadcast.py:519— theother_instancescache is keyed by service name andon_remove(name)resolves the cached instance (a removed service can't be re-queried); confirmreset_broadcasting()there lets a peer leave→rejoin re-publish an ADD.kolibri/utils/server/__init__.py:322—run()now callsupdate_broadcast()unconditionally each tick and the transport detects the address change internally; confirm theLISTEN_ADDRESS != "0.0.0.0"gate (dummy monitor,frequency=-1) still skips dynamic updates for specific-address deployments.kolibri/core/discovery/utils/network/broadcast.py:434—start_broadcastcallstransport.register()beforetransport.start_listening()so the transport stores its own instance before any peer event fires; check this ordering contract holds for an override transport.AI usage
Used Claude Code to implement the extraction from a human-reviewed implementation plan, following the existing overridable-default hook mechanism. Verified with the discovery and server test suites and ruff.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
🟡 Waiting for feedback
Last updated: 2026-07-17 22:11 UTC