fleetd #453: document the override obligation on PeerLauncher.defaultProfileFor/place
Decision: leave both as default methods (option 1), not abstract. Neither default is a live defect today — HerdrPeerLauncher is the sole single-profile implementer and the degenerate answer (ignore role, always defaultProfile()) is correct for it. Making them abstract would force ~10 boilerplate one-line overrides across 5 unrelated PeerLauncher test doubles (NeverSpawnsLauncher, RaceLauncher, NoResumeLauncher, ClearContextSpyLauncher, LazyIdLauncher) that never call either method, for a risk that is speculative (no multi-profile HerdrPeerLauncher subclass exists or is planned). Strengthens both javadocs with an explicit MUST-override warning and cross- references HerdrPeerLauncher.spawn(SpawnRequest, PlacementDecision)'s existing #450 javadoc, which already names both methods as "unoverridden here" and ties that to being a single-profile adapter -- the concrete place a future multi-profile launcher author would read, since #450 made that method abstract and any subclass must write its body.
This commit is contained in:
@@ -168,6 +168,16 @@ public interface PeerLauncher {
|
||||
* answer for a launcher with no role-pool concept of its own (e.g. a single {@code
|
||||
* HerdrPeerLauncher} adapter, which is never reached this way in production: {@code
|
||||
* CompositePeerLauncher} always fronts it and resolves roles itself).
|
||||
*
|
||||
* <p>fleetd #453: this default is deliberately <em>not</em> abstract — unlike {@link
|
||||
* #spawn(SpawnRequest, PlacementDecision)} (fleetd #450), there is no live defect in inheriting
|
||||
* it today, and the only current single-adapter implementer ({@code HerdrPeerLauncher}) is
|
||||
* correct to do so. But it stays correct only as long as that holds: <strong>if a launcher ever
|
||||
* routes more than one profile per role, it MUST override this method</strong>, or every role
|
||||
* silently resolves to {@link #defaultProfile()} with no error and no log line. {@code
|
||||
* HerdrPeerLauncher}'s own {@link #spawn(SpawnRequest, PlacementDecision)} javadoc names this
|
||||
* method and {@link #place} explicitly as "unoverridden here" for exactly this reason — read
|
||||
* that javadoc before adding role-pool routing to any {@code HerdrPeerLauncher} subclass.
|
||||
*/
|
||||
default String defaultProfileFor(MemberRole role) {
|
||||
return defaultProfile();
|
||||
@@ -228,6 +238,13 @@ public interface PeerLauncher {
|
||||
* placement condition — the right answer for a launcher with no pool or placement-policy
|
||||
* concept of its own, matching {@link #defaultProfileFor}'s own default.
|
||||
*
|
||||
* <p>fleetd #453: same reasoning as {@link #defaultProfileFor}'s own #453 note — this default
|
||||
* is deliberately not abstract (no live defect today, correct for the sole single-adapter
|
||||
* implementer), but <strong>a launcher that ever routes more than one profile per role MUST
|
||||
* override this method too</strong>, or placement silently ignores {@code role} for it. See
|
||||
* {@code HerdrPeerLauncher.spawn(SpawnRequest, PlacementDecision)}'s javadoc, which names this
|
||||
* method as "unoverridden here" and why that is correct only for a single-profile adapter.
|
||||
*
|
||||
* @throws RuntimeException (implementation-specific, typically a placement exception) if no
|
||||
* candidate in {@code role}'s pool is currently placeable
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user