feat: pluggable session supervisor and :via tuple session naming - #133
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughProblemAnubis hardcodes Solution
RationaleMaintains backward compatibility while enabling distributed deployments. Follows the existing registry pluggability model and co-locates naming strategy with the registry adapter, eliminating the race condition between WalkthroughThis change introduces configurable session naming and session supervisor selection in the Anubis server. The registry behavior now defines an optional Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
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 |
Problem
Anubis.Server.SupervisorhardcodedDynamicSupervisorin three places, making it impossible to swap inHorde.DynamicSupervisorfor distributed deployments. Additionally,Registry.session_name/2returned plain atoms, preventing Horde.Registry from auto-registering sessions via:viatuples — creating a race condition window betweenstart_childand explicitregister_sessioncalls.Fixes #113.
Solution
Option A — Pluggable session supervisor
Added a
:supervisoroption toAnubis.Server.Supervisor.start_link/2, mirroring the existing:registryoption:The resolved module is stored in
persistent_termand used instart_session/2andstop_session/3. Defaults to{DynamicSupervisor, []}— fully backward compatible.Option B1 —
session_name/2callback on the Registry behaviourAdded an optional
session_name/2callback toAnubis.Server.Registry:Both
Registry.LocalandRegistry.Noneimplement it returning atoms (same behaviour as before). A Horde adapter can return{:via, Horde.Registry, {name, session_id}}, which causes auto-registration onGenServer.start_link— makingregister_session/3a no-op and eliminating the race condition entirely.Registry.resolve_session_name/3is the new call site in both the StreamableHTTP plug and the SSE transport, falling back to atom naming when the adapter doesn't implement the callback.Rationale
Followed the existing
:registrypluggability pattern for:supervisor— minimal surface, same mental model. Thesession_name/2callback keeps naming strategy co-located with the registry adapter that owns it, rather than spreading the logic across transports. Making it@optional_callbackspreserves backward compatibility for existing custom adapters.