Skip to content

Commit 093ff75

Browse files
SEP-1818: Warn when the chosen executor node is not the node hosting the target service (#1399)
## Summary Added an advisory executor-host mismatch warning so SEP task forms can surface when the selected executor runs on a different node than the selected service, without changing submission or cascade behavior. - `app/sep/apps/framework/form_dsl/markers.py`, `app/sep/apps/framework/form_dsl/derivation.py`, `app/sep/apps/framework/schema.py`, `app/sep/apps/framework/form_dsl/model.py`, `app/sep/apps/alters/models.py`: added the new `target_service` form-DSL/schema contract, derived it onto host fields, and declared `service_id` as the target service for executor host fields. - `frontend/packages/framework/src/components/HostSelector/HostSelector.tsx`, `frontend/packages/framework/src/components/HostSelector/isHostMismatch.ts`, `frontend/packages/framework/src/components/SchemaFormRenderer/fields/HostField.tsx`, `frontend/packages/api/src/types/app-schema.ts`: added address-based mismatch detection, rendered a non-blocking warning in the single host selector, and threaded the new schema field through the frontend types and renderer. - `frontend/packages/api/specs/sep.json`, `frontend/packages/api/src/generated/sep.ts`, `tests/app/sep/snapshots/openapi/*.json`, `tests/app/sep/snapshots/schema/*.json`: regenerated the OpenAPI spec, generated TypeScript client, and schema/openapi snapshots for the additive `target_service` contract. - `tests/app/sep/apps/framework/test_form_dsl.py`, `tests/app/sep/apps/framework/test_task_form_model.py`, `frontend/packages/framework/src/components/HostSelector/isHostMismatch.test.ts`, `frontend/packages/framework/src/components/HostSelector/HostSelector.test.tsx`: added backend/frontend coverage for derivation rules, task-form propagation, mismatch comparison, warning rendering, and silence states. - Test files: updated focused backend and frontend tests to cover the new warning behavior and preserve existing host selector behavior. ## Tested - [x] Show a warning when the host and service are on different nodes, and hide it when they match. - [x] Do not show the warning when the service is missing or the check cannot run. - [x] Keep MongoDB Backup cascade working, and warn only if the user picks a different host. ## Checklist <!-- Check items that apply. Leave unchecked items visible — reviewers use this too. --> - [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 (`make test`) - [x] Pre-commit hooks pass (`make run-pre-commit`) - [ ] Database migrations generated if models changed (`make makemigrations`) - [ ] User-facing changes documented (README, inline help, UI text) - [ ] Configuration changes documented with examples - [ ] Changelog fragment added under `changelog.d/` if the change is user-facing (`make changelog-add`), or confirmed N/A (internal-only change, or a same-release-cycle fix for an unreleased sibling ticket) --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 1b17222 commit 093ff75

39 files changed

Lines changed: 1316 additions & 38 deletions

app/sep/apps/alters/models.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,9 @@ def _default_dsn_table_for_dsn_recursion(cls, data: Any) -> Any:
128128

129129
task_name: Annotated[NonEmptyStr, Ui(label="Task Name", section="Task")]
130130
hostname: Annotated[
131-
NonEmptyStr, HostRef(), Ui(label=EXECUTION_HOST_LABEL, section="Task")
131+
NonEmptyStr,
132+
HostRef(target_service="service_id"),
133+
Ui(label=EXECUTION_HOST_LABEL, section="Task"),
132134
]
133135
service_id: Annotated[
134136
int,

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,12 @@ def _build_ref_field(
462462
# MultiHostField may carry the key for wire uniformity, but only the
463463
# single-value HostField renderer honours it today.
464464
return field_class(
465-
**common, allow_custom=allow_custom, depends_on=ui.depends_on
465+
**common,
466+
allow_custom=allow_custom,
467+
depends_on=ui.depends_on,
468+
target_service=(
469+
ref.target_service if ref.target_service is not None else ui.depends_on
470+
),
466471
)
467472
return field_class(**common, allow_custom=allow_custom)
468473

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,15 +312,26 @@ class HostRef:
312312
(``multiple=True``) may still emit ``depends_on`` on ``MultiHostField``,
313313
but cascade auto-select is single-host only today.
314314
315+
``target_service`` is independent of ``Ui(depends_on=...)``: setting it does
316+
not turn on cascade auto-select. It names the service field whose node
317+
address the renderer compares against the selected host for a non-blocking
318+
co-location warning. When it is omitted, derivation falls back to
319+
``Ui(depends_on=...)`` when that is set, so a form that already declares a
320+
service-driven cascade gets the warning with no new declaration.
321+
315322
:param allow_custom: When ``True``, the field also accepts a free-typed
316323
value and emits ``allow_custom`` on the wire. Defaults to ``False``.
317324
:param multiple: When ``True``, the field is a multi-value selector backed by
318325
a ``list[...]`` / ``set[...]`` annotation and derives a
319326
``MultiHostField``. Defaults to ``False`` (single-value).
327+
:param target_service: Optional name of the service field used for the
328+
co-location warning. ``None`` (the default) lets derivation fall back
329+
to ``Ui(depends_on=...)`` when that is set.
320330
"""
321331

322332
allow_custom: bool = False
323333
multiple: bool = False
334+
target_service: str | None = None
324335

325336

326337
@dataclass(frozen=True, slots=True)

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,15 @@ class TaskFormModel(AppFormModel):
8787
8888
:param task_name: The human-readable task name; required and non-empty.
8989
:param hostname: The executor host the task runs on; required and non-empty.
90+
The default ``HostRef(target_service="service_id")`` ties the
91+
co-location warning to a service field named ``service_id``; subclasses
92+
that name their service field differently must override with an explicit
93+
``HostRef(target_service=...)``.
9094
"""
9195

9296
task_name: Annotated[NonEmptyStr, Ui(section="Task")]
9397
hostname: Annotated[
94-
NonEmptyStr, HostRef(), Ui(label=EXECUTION_HOST_LABEL, section="Task")
98+
NonEmptyStr,
99+
HostRef(target_service="service_id"),
100+
Ui(label=EXECUTION_HOST_LABEL, section="Task"),
95101
]

app/sep/apps/framework/schema.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,10 +462,13 @@ class HostField(BaseField):
462462
463463
:param field_type: The discriminator literal; always ``"host"`` for this
464464
class. Serialised as the JSON key ``"type"``.
465-
:type field_type: Literal["host"]
466465
:param depends_on: Optional name of the field whose value drives the
467466
default executor selection. ``None`` (the default) omits the key from
468467
the wire so plugins that do not opt in stay byte-identical.
468+
:param target_service: Optional service field for a non-blocking
469+
co-location warning. Independent of ``depends_on`` (see
470+
:class:`~app.sep.apps.framework.form_dsl.markers.HostRef`).
471+
``None`` (the default) omits the key from the wire.
469472
:param allow_custom: When ``True``, the selector also accepts a free-typed
470473
value alongside the inventory options. ``None`` (the default) omits the
471474
key from the wire so plugins that do not opt in stay byte-identical.
@@ -475,6 +478,7 @@ class HostField(BaseField):
475478
"host", alias="type", serialization_alias="type"
476479
)
477480
depends_on: NonEmptyStr | None = None
481+
target_service: NonEmptyStr | None = None
478482
allow_custom: bool | None = None
479483

480484

@@ -488,13 +492,18 @@ class MultiHostField(BaseField):
488492
Cascade auto-select is single-host only (:class:`HostField`). ``depends_on``
489493
may still be emitted when ``Ui(depends_on=...)`` is set so derivation stays
490494
uniform, but the multi-host renderer does not honour it today.
495+
``target_service`` is mirrored the same way; the multi-host renderer
496+
ignores it (no co-location warning).
491497
492498
:param field_type: The discriminator literal; always ``"multi_host"`` for
493499
this class. Serialised as the JSON key ``"type"``.
494500
:param depends_on: Optional upstream field name mirrored from
495501
``Ui(depends_on=...)``. Emitted for wire uniformity with
496502
:class:`HostField`; the current multi-host renderer ignores it (no
497503
cascade). ``None`` (the default) omits the key from the wire.
504+
:param target_service: Optional service field name mirrored for wire
505+
uniformity with :class:`HostField`; ignored by the multi-host
506+
renderer. ``None`` (the default) omits the key from the wire.
498507
:param allow_custom: When ``True``, the selector also accepts free-typed
499508
values alongside the inventory options. ``None`` (the default) omits the
500509
key from the wire so plugins that do not opt in stay byte-identical.
@@ -504,6 +513,7 @@ class MultiHostField(BaseField):
504513
"multi_host", alias="type", serialization_alias="type"
505514
)
506515
depends_on: NonEmptyStr | None = None
516+
target_service: NonEmptyStr | None = None
507517
allow_custom: bool | None = None
508518

509519

changelog.d/SEP-1818.changed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Task create forms that pick an execution host against a database service now show a non-blocking warning when the selected host is not the node where that service runs; submit stays allowed.

frontend/packages/api/specs/sep.json

Lines changed: 26 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/packages/api/src/generated/sep.ts

Lines changed: 13 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/packages/api/src/types/app-schema.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,11 @@ export interface HostField extends BaseField {
215215
* not cascaded.
216216
*/
217217
depends_on?: string;
218+
/**
219+
* Service field for a non-blocking co-location warning. Independent of
220+
* ``depends_on``. Omitted when unset.
221+
*/
222+
target_service?: string;
218223
/** Offer free-text (free-solo) entry alongside the inventory options. */
219224
allow_custom?: boolean;
220225
}
@@ -227,6 +232,11 @@ export interface MultiHostField extends BaseField {
227232
* today. Omitted when unset.
228233
*/
229234
depends_on?: string;
235+
/**
236+
* Optional service field name mirrored for wire uniformity. The
237+
* multi-host renderer ignores it. Omitted when unset.
238+
*/
239+
target_service?: string;
230240
/** Offer free-text (free-solo) entry alongside the inventory options. */
231241
allow_custom?: boolean;
232242
}

0 commit comments

Comments
 (0)