Skip to content

Commit ea4d265

Browse files
SEP-1443: Decouple primary-service selection from connectivity-probe enablement (#1131)
## Summary - Add a per-reference `primary=True` marker to `ServiceRef` so an app can designate the envelope primary **independently** of `check_connectivity`. Previously `check_connectivity=True` did double duty — naming the primary *and* enabling the create-route connectivity probe — so there was no way to express "multiple services, one designated primary, no probe". - `check_connectivity=True` keeps implying primary; probe enablement (`connectivity_check`) stays keyed off `check_connectivity` alone. Existing single-marker apps are byte-for-byte unchanged. - `_validate_connectivity_refs` now accepts 2+ `ServiceRef` fields when exactly one primary is designated (via either marker) and rejects conflicting/duplicate designations with a clear `ValueError`. No app consumes the new marker yet — opt-in, no DB migration, no wire/schema change. ## Tested - [x] `pytest tests/app/sep/apps/framework/` → 1590 passed, 234 skipped, 0 failed - [x] Golden-file / OpenAPI / contract snapshots byte-identical (no regeneration needed — wire format unchanged) - [x] `ruff check` + `ruff format --check` clean on all changed files - New unit tests cover: primary designation without probe, primary beats last-resolved selection, unfilled-primary → no primary (missing-service guard), and the four validation rejections (primary+check conflict, two primaries, primary+multiple, redundant same-field allowed). ## Checklist - [x] New/modified functions have type hints and rST docstrings - [x] New tests added for new features or bug fixes - [x] All tests pass locally (framework suite; full `make test` not re-run) - [x] Pre-commit hooks pass (ruff/format verified on changed files) - [ ] Database migrations generated if models changed (`make makemigrations`) — N/A, no model/table change - [ ] User-facing changes documented — N/A, marker is opt-in and unused by any app - [ ] Configuration changes documented — N/A - [x] Changelog fragment added under `changelog.d/`
1 parent 3dbe951 commit ea4d265

5 files changed

Lines changed: 250 additions & 48 deletions

File tree

app/sep/apps/framework/apps.py

Lines changed: 39 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -455,8 +455,9 @@ def connectivity_check(self) -> bool:
455455
An app probes iff the create capability is enabled and its ``create_model``
456456
declares a ``check_connectivity=True`` ``ServiceRef`` (top-level or nested
457457
in a one-of branch); that marked service is also the envelope's primary.
458-
Replaces the former app-level flag, so a single per-reference marker drives
459-
both the probe and the connectivity-service selection.
458+
Probe enablement is keyed off ``check_connectivity`` alone — a ``ServiceRef``
459+
marked ``primary=True`` designates the envelope primary without probing, so it
460+
does not turn this on.
460461
461462
:return: ``True`` when the app derives a probing create route.
462463
"""
@@ -538,57 +539,58 @@ def _validate_related_apps(self) -> None:
538539
)
539540

540541
def _validate_connectivity_refs(self) -> None:
541-
"""Reject an ambiguous or unselectable connectivity-service configuration.
542-
543-
A model-first app probes the ``check_connectivity=True`` ``ServiceRef`` and
544-
makes it the envelope's primary. At most one service may be marked — a
545-
second would make the probe target ambiguous — and a model declaring two or
546-
more ``ServiceRef`` fields with none marked has no determinable primary. A
547-
single unmarked ``ServiceRef`` is valid: it is the sole primary and the app
548-
does not probe. The two-or-more-unmarked rejection is about primary
549-
disambiguation, not the probe: ``assemble_envelope`` unconditionally stamps
550-
the primary service onto every task's connectivity host/port and
542+
"""Reject an ambiguous or unselectable primary-service configuration.
543+
544+
A model-first app names its envelope primary with a *designated* marker —
545+
``check_connectivity=True`` (which also probes) or ``primary=True`` (which
546+
designates without probing). At most one ``ServiceRef`` may be designated
547+
across both markers; a second designation makes the primary ambiguous. A
548+
model declaring two or more ``ServiceRef`` fields with none designated has
549+
no determinable primary. A single unmarked ``ServiceRef`` is valid: it is
550+
the sole primary and the app does not probe. The disambiguation is about the
551+
primary, not the probe: ``assemble_envelope`` unconditionally stamps the
552+
primary service onto every task's connectivity host/port and
551553
``service_name``, so the primary must be unambiguous even when no probe runs.
552554
553-
:raises ValueError: When a ``check_connectivity`` ``ServiceRef`` is also
554-
marked ``multiple`` (a multi-value field has no single primary), when a
555-
``multiple=True`` ``ServiceRef`` would be the connectivity primary (no
556-
scalar ``check_connectivity`` ref is marked to take its place), when
557-
more than one ``ServiceRef`` is marked ``check_connectivity``, or when
558-
two or more ``ServiceRef`` fields leave no determinable connectivity
559-
primary.
555+
:raises ValueError: When a designated ``ServiceRef`` is also marked
556+
``multiple`` (a multi-value field has no single primary), when a
557+
``multiple=True`` ``ServiceRef`` would be the primary with no scalar
558+
designated ref to take its place, when more than one ``ServiceRef`` is
559+
designated primary (via ``check_connectivity`` and/or ``primary``), or
560+
when two or more ``ServiceRef`` fields leave no determinable primary.
560561
"""
561562
if self.create_model is None:
562563
return
563564
refs = list(iter_service_refs(self.create_model))
564-
marked = [ref for ref in refs if ref.check_connectivity]
565-
if any(ref.multiple for ref in marked):
565+
designated = [ref for ref in refs if ref.check_connectivity or ref.primary]
566+
if any(ref.multiple for ref in designated):
566567
raise ValueError(
567568
"TaskExecutionApp: a create_model declares a multiple=True ServiceRef "
568-
"marked check_connectivity=True; the connectivity probe targets a "
569-
"single primary service and cannot select one from a multi-value "
570-
"field — set multiple=False or check_connectivity=False"
569+
"designated primary (check_connectivity=True or primary=True); the "
570+
"envelope primary is a single service and cannot be selected from a "
571+
"multi-value field — set multiple=False, or drop the primary marker"
571572
)
572-
if not marked and any(ref.multiple for ref in refs):
573+
if not designated and any(ref.multiple for ref in refs):
573574
raise ValueError(
574575
"TaskExecutionApp: a create_model declares a multiple=True ServiceRef "
575-
"with no check_connectivity=True ServiceRef to serve as the "
576-
"connectivity primary; a multi-value service field cannot resolve to "
577-
"the single primary assemble_envelope stamps onto every task — mark a "
578-
"scalar ServiceRef check_connectivity=True"
576+
"with no designated primary ServiceRef to serve as the envelope "
577+
"primary; a multi-value service field cannot resolve to the single "
578+
"primary assemble_envelope stamps onto every task — mark a scalar "
579+
"ServiceRef check_connectivity=True or primary=True"
579580
)
580-
if len(marked) > 1:
581+
if len(designated) > 1:
581582
raise ValueError(
582-
"TaskExecutionApp: a create_model declares "
583-
f"{len(marked)} check_connectivity=True ServiceRef fields; at most "
584-
"one service is the connectivity primary — mark exactly one"
583+
"TaskExecutionApp: a create_model designates "
584+
f"{len(designated)} primary ServiceRef fields via check_connectivity "
585+
"and/or primary; at most one service is the envelope primary — "
586+
"designate exactly one"
585587
)
586-
if not marked and len(refs) > 1:
588+
if not designated and len(refs) > 1:
587589
raise ValueError(
588590
"TaskExecutionApp: a create_model declares "
589-
f"{len(refs)} ServiceRef fields with none marked "
590-
"check_connectivity=True; no connectivity primary is determinable — "
591-
"mark exactly one"
591+
f"{len(refs)} ServiceRef fields with none designated primary "
592+
"(check_connectivity=True or primary=True); no envelope primary "
593+
"is determinable — designate exactly one"
592594
)
593595

594596
def _validate_schema_source(self) -> None:

app/sep/apps/framework/form_dsl/markers.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,17 @@ class ServiceRef:
230230
post-creation connectivity probe against this service, and the service is
231231
selected as the envelope's primary (``_service_name``, the connectivity
232232
meta, and the executor-target fallback) even when a second ``ServiceRef``
233-
resolves. A model declares at most one ``check_connectivity`` service;
234-
when none is marked the sole ``ServiceRef`` is the primary and no probe
235-
runs. Defaults to ``False``.
233+
resolves. ``check_connectivity`` therefore implies ``primary``. When none
234+
is marked the sole ``ServiceRef`` is the primary and no probe runs.
235+
Defaults to ``False``.
236+
:param primary: When ``True``, the service is the envelope's primary
237+
(``_service_name``, the connectivity meta, and the executor-target
238+
fallback) *without* enabling the probe — the way to name a primary among
239+
several ``ServiceRef`` fields when no connectivity check is wanted. A model
240+
designates at most one primary across both markers: at most one
241+
``ServiceRef`` may be marked ``check_connectivity`` **or** ``primary`` (a
242+
single field carrying both is redundant, since ``check_connectivity``
243+
already implies primary). Defaults to ``False``.
236244
:param multiple: When ``True``, the field is a multi-value selector backed by
237245
a ``list[...]`` / ``set[...]`` annotation and derives a
238246
``MultiServiceField``. Defaults to ``False`` (single-value).
@@ -241,6 +249,7 @@ class ServiceRef:
241249
service_types: tuple[ServiceTypeEnum, ...]
242250
allow_custom: bool = False
243251
check_connectivity: bool = False
252+
primary: bool = False
244253
multiple: bool = False
245254

246255
def __post_init__(self) -> None:

app/sep/apps/framework/spec.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -288,18 +288,18 @@ def _select_primary_service(
288288
) -> CreatedService | None:
289289
"""Select the primary (connectivity) service from the resolved service refs.
290290
291-
A ``check_connectivity``-marked ref wins outright (the construction guard
292-
permits at most one). Otherwise fall back to the last ref that resolved to a
293-
real entity, preserving the pre-marker last-wins behaviour for the
294-
single-service apps that declare no marker.
291+
A designated primary — a ref marked ``check_connectivity`` or ``primary`` —
292+
wins outright (the construction guard permits at most one). Otherwise fall
293+
back to the last ref that resolved to a real entity, preserving the pre-marker
294+
last-wins behaviour for the single-service apps that declare no marker.
295295
296296
:param candidates: The ``(marker, resolved entity)`` pairs for every resolved
297297
``ServiceRef`` field, in declaration order.
298298
:return: The primary service, or ``None`` when none resolved.
299299
"""
300300
primary = None
301301
for ref, entity in candidates:
302-
if ref.check_connectivity:
302+
if ref.check_connectivity or ref.primary:
303303
return entity
304304
if entity is not None:
305305
primary = entity
@@ -319,9 +319,9 @@ async def resolve_refs(
319319
spec builder can fall back to the raw form value. A ``HostRef`` field's
320320
submitted value (free-typed or selected) is captured as the executor host
321321
without an inventory call, coerced to ``str``; a model declaring more than one
322-
``HostRef`` is rejected. The connectivity / primary service is the
323-
``check_connectivity``-marked ``ServiceRef`` when present, else the sole
324-
resolved ``ServiceRef``.
322+
``HostRef`` is rejected. The primary service is the designated ``ServiceRef``
323+
(marked ``check_connectivity`` or ``primary``) when present, else the sole /
324+
last-resolved ``ServiceRef``.
325325
326326
:param form: The validated create form instance.
327327
:param inventory_api: The inventory API client.

tests/app/sep/apps/framework/test_apps.py

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,90 @@ class _SoleMultiServiceForm(AppFormModel):
181181
]
182182

183183

184+
class _PrimaryDesignatedServiceForm(AppFormModel):
185+
"""Represent two services, one designated ``primary`` without a probe.
186+
187+
The primary is declared *before* an unmarked destination so the old
188+
two-or-more-unmarked rejection no longer fires and the designation, not
189+
last-wins, names the primary.
190+
"""
191+
192+
task_name: Annotated[str, Ui(label="Name", section="main")] = ""
193+
service_a: Annotated[
194+
int | None,
195+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,), primary=True),
196+
Ui(label="A", section="main"),
197+
] = None
198+
service_b: Annotated[
199+
int | None,
200+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,)),
201+
Ui(label="B", section="main"),
202+
] = None
203+
204+
205+
class _PrimaryAndCheckConflictForm(AppFormModel):
206+
"""Represent a ``primary`` field conflicting with a ``check_connectivity`` field."""
207+
208+
task_name: Annotated[str, Ui(label="Name", section="main")] = ""
209+
service_a: Annotated[
210+
int | None,
211+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,), primary=True),
212+
Ui(label="A", section="main"),
213+
] = None
214+
service_b: Annotated[
215+
int | None,
216+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,), check_connectivity=True),
217+
Ui(label="B", section="main"),
218+
] = None
219+
220+
221+
class _TwoPrimaryServiceForm(AppFormModel):
222+
"""Represent a create form designating two ``primary`` services."""
223+
224+
task_name: Annotated[str, Ui(label="Name", section="main")] = ""
225+
service_a: Annotated[
226+
int | None,
227+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,), primary=True),
228+
Ui(label="A", section="main"),
229+
] = None
230+
service_b: Annotated[
231+
int | None,
232+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,), primary=True),
233+
Ui(label="B", section="main"),
234+
] = None
235+
236+
237+
class _PrimaryMultipleForm(AppFormModel):
238+
"""Represent a multi-value ``ServiceRef`` designated ``primary``."""
239+
240+
task_name: Annotated[str, Ui(label="Name", section="main")] = ""
241+
services: Annotated[
242+
list[int],
243+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,), multiple=True, primary=True),
244+
Ui(label="Services", section="main"),
245+
]
246+
247+
248+
class _RedundantPrimaryProbeForm(AppFormModel):
249+
"""Represent a single field carrying both ``primary`` and ``check_connectivity``."""
250+
251+
task_name: Annotated[str, Ui(label="Name", section="main")] = ""
252+
service_a: Annotated[
253+
int | None,
254+
ServiceRef(
255+
service_types=(ServiceTypeEnum.MYSQL,),
256+
check_connectivity=True,
257+
primary=True,
258+
),
259+
Ui(label="A", section="main"),
260+
] = None
261+
service_b: Annotated[
262+
int | None,
263+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,)),
264+
Ui(label="B", section="main"),
265+
] = None
266+
267+
184268
class _BadArgFormatForm(AppFormModel):
185269
"""Represent a create form whose ``ArgFormat`` template misspells the placeholder."""
186270

@@ -870,6 +954,39 @@ def test_marked_service_enables_probe(self) -> None:
870954
"""Derive ``connectivity_check`` from a ``check_connectivity`` service."""
871955
assert _synth_app(connectivity_check=True).connectivity_check is True
872956

957+
def test_primary_marker_designates_without_probe(self) -> None:
958+
"""Accept two services with one ``primary`` designation and no probe."""
959+
assert (
960+
_synth_app(create_model=_PrimaryDesignatedServiceForm).connectivity_check
961+
is False
962+
)
963+
964+
def test_primary_and_check_connectivity_conflict_raises(self) -> None:
965+
"""Reject a ``primary`` field conflicting with a ``check_connectivity`` field."""
966+
with pytest.raises(
967+
ValueError, match="at most one service is the envelope primary"
968+
):
969+
_synth_app(create_model=_PrimaryAndCheckConflictForm)
970+
971+
def test_two_primary_designations_raise(self) -> None:
972+
"""Reject a create_model designating more than one ``primary`` service."""
973+
with pytest.raises(
974+
ValueError, match="at most one service is the envelope primary"
975+
):
976+
_synth_app(create_model=_TwoPrimaryServiceForm)
977+
978+
def test_primary_with_multiple_raises(self) -> None:
979+
"""Reject a multiple=True ServiceRef designated the primary."""
980+
with pytest.raises(ValueError, match="multiple=True"):
981+
_synth_app(create_model=_PrimaryMultipleForm)
982+
983+
def test_redundant_primary_and_probe_same_field_allowed(self) -> None:
984+
"""Accept one field marked both ``primary`` and ``check_connectivity``, probing."""
985+
assert (
986+
_synth_app(create_model=_RedundantPrimaryProbeForm).connectivity_check
987+
is True
988+
)
989+
873990
def test_list_suppress_without_custom_list_raises(self) -> None:
874991
"""Assert ``list=False`` with no custom ``GET /`` in extra_routes is rejected."""
875992
with pytest.raises(ValueError, match="capabilities.list"):

tests/app/sep/apps/framework/test_spec.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,27 @@ class _SoleUnmarkedServiceForm(AppFormModel):
411411
] = None
412412

413413

414+
class _PrimaryDesignatedServiceForm(AppFormModel):
415+
"""Carry a ``primary`` source and an unmarked destination ``ServiceRef``.
416+
417+
The destination is declared *after* the source so the old last-wins
418+
selection would pick it; the ``primary`` marker must instead keep the source
419+
as ``resolved.service`` without enabling any probe.
420+
"""
421+
422+
task_name: Annotated[str, Ui(label="Name", section="main")] = ""
423+
source_id: Annotated[
424+
int | None,
425+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,), primary=True),
426+
Ui(label="Source", section="main"),
427+
] = None
428+
dest_id: Annotated[
429+
int | None,
430+
ServiceRef(service_types=(ServiceTypeEnum.MYSQL,)),
431+
Ui(label="Dest", section="main"),
432+
] = None
433+
434+
414435
class _SourceByTable(BaseModel):
415436
"""Carry a discriminated-union branch nesting a free-solo ``SchemaRef``."""
416437

@@ -598,6 +619,59 @@ async def test_check_connectivity_service_is_primary(self) -> None:
598619
assert resolved.entities["source_id"].name == "source"
599620
assert resolved.entities["dest_id"].name == "dest"
600621

622+
@pytest.mark.asyncio
623+
async def test_primary_marked_service_is_primary_without_probe(self) -> None:
624+
"""Select the ``primary`` service as primary, not the last-resolved ref."""
625+
source = _service(
626+
address="src-host",
627+
service_type=ServiceTypeEnum.MYSQL,
628+
name="source",
629+
port=3306,
630+
)
631+
dest = _service(
632+
address="dst-host",
633+
service_type=ServiceTypeEnum.MYSQL,
634+
name="dest",
635+
port=3306,
636+
)
637+
inventory = _fake_inventory(
638+
{
639+
"/services/1": source.model_dump(mode="json"),
640+
"/services/2": dest.model_dump(mode="json"),
641+
}
642+
)
643+
644+
resolved = await resolve_refs(
645+
_PrimaryDesignatedServiceForm(source_id=1, dest_id=2), inventory
646+
)
647+
648+
assert resolved.service is not None
649+
assert resolved.service.name == "source"
650+
assert resolved.entities["dest_id"].name == "dest"
651+
652+
@pytest.mark.asyncio
653+
async def test_unfilled_primary_designation_yields_no_primary(self) -> None:
654+
"""Return no primary when the designated ``primary`` ref is left unfilled.
655+
656+
The designated ref wins outright even when it resolves to ``None``, so a
657+
later resolved ``ServiceRef`` does not step in as the primary — leaving
658+
``assemble_envelope`` to trip its missing-service guard.
659+
"""
660+
dest = _service(
661+
address="dst-host",
662+
service_type=ServiceTypeEnum.MYSQL,
663+
name="dest",
664+
port=3306,
665+
)
666+
inventory = _fake_inventory({"/services/2": dest.model_dump(mode="json")})
667+
668+
resolved = await resolve_refs(
669+
_PrimaryDesignatedServiceForm(source_id=None, dest_id=2), inventory
670+
)
671+
672+
assert resolved.service is None
673+
assert resolved.entities["dest_id"].name == "dest"
674+
601675
@pytest.mark.asyncio
602676
async def test_sole_service_ref_is_primary_when_none_marked(self) -> None:
603677
"""Select the sole ``ServiceRef`` as primary when none is marked."""

0 commit comments

Comments
 (0)