Skip to content

Commit 2b86496

Browse files
committed
refactor(domain): operator-visible deprecation for instance_type/instance_profile
The instance_type->machine_type and instance_profile->machine_role aliases only warned on the Python-kwarg path; the model_validate / YAML deserialization path that operators actually use accepted the old key silently (AliasChoices), and the repo's warnings filter suppresses DeprecationWarning even in tests. Move the deprecation signal to a model_validator(mode="before") that emits logger.warning on every deserialization path, and mark the new fields with Field(deprecated=...) for OpenAPI/schema visibility. The application-layer command handler emits a single operator-visible warning at the boundary where instance_type is mapped to machine_types. This establishes the house pattern for operator-facing Pydantic field deprecations: AliasChoices for back-compat + model_validator(mode=before) logger.warning for the runtime signal + Field(deprecated=) for schema.
1 parent f4fd4dd commit 2b86496

4 files changed

Lines changed: 251 additions & 10 deletions

File tree

src/orb/application/commands/template_handlers.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
"""Template command handlers for CQRS pattern."""
22

3+
import logging
4+
35
from orb.application.base.handlers import BaseCommandHandler
6+
7+
_logger = logging.getLogger(__name__)
48
from orb.application.decorators import command_handler
59
from orb.application.template.commands import (
610
CreateTemplateCommand,
@@ -98,6 +102,9 @@ async def execute_command(self, command: CreateTemplateCommand) -> None:
98102
}
99103
# instance_type → machine_types (backward compat)
100104
if command.instance_type is not None and "machine_types" not in dto_fields:
105+
_logger.warning(
106+
"CreateTemplateCommand.instance_type is deprecated; use machine_type instead."
107+
)
101108
dto_fields["machine_types"] = {command.instance_type: 1}
102109
# Remove None values so TemplateDTO defaults apply
103110
dto_fields = {k: v for k, v in dto_fields.items() if v is not None}
@@ -171,6 +178,9 @@ async def execute_command(self, command: UpdateTemplateCommand) -> None:
171178

172179
# instance_type → machine_types (backward compat)
173180
if command.instance_type is not None and "machine_types" not in update_fields:
181+
_logger.warning(
182+
"UpdateTemplateCommand.instance_type is deprecated; use machine_type instead."
183+
)
174184
update_fields["machine_types"] = {command.instance_type: 1}
175185

176186
if update_fields:

src/orb/domain/template/template_aggregate.py

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
"""Template configuration value object - core template domain logic."""
22

3+
import logging
34
import warnings
45
from datetime import datetime
56
from typing import Any, Optional
67

78
from pydantic import AliasChoices, BaseModel, ConfigDict, Field, model_validator
89

10+
logger = logging.getLogger(__name__)
11+
912

1013
class Template(BaseModel):
1114
"""Template configuration value object with both snake_case and camelCase support via aliases."""
@@ -25,6 +28,7 @@ class Template(BaseModel):
2528
machine_type: Optional[str] = Field(
2629
default=None,
2730
validation_alias=AliasChoices("machine_type", "instance_type"),
31+
deprecated="use 'machine_type' instead of 'instance_type'",
2832
)
2933
image_id: Optional[str] = None
3034
max_instances: int = 1
@@ -61,6 +65,7 @@ class Template(BaseModel):
6165
machine_role: Optional[str] = Field(
6266
default=None,
6367
validation_alias=AliasChoices("machine_role", "instance_profile"),
68+
deprecated="use 'machine_role' instead of 'instance_profile'",
6469
) # IAM role, service principal, or service account
6570

6671
# Advanced configuration (extensible)
@@ -82,6 +87,42 @@ class Template(BaseModel):
8287
# Active status flag
8388
is_active: bool = True
8489

90+
@model_validator(mode="before")
91+
@classmethod
92+
def _warn_deprecated_field_names(cls, data: Any) -> Any:
93+
"""House pattern for operator-facing Pydantic field deprecation.
94+
95+
This is the canonical way to emit operator-visible deprecation warnings
96+
for renamed fields in this codebase. It runs on the raw input dict
97+
before Pydantic applies AliasChoices, so it fires on EVERY entry point:
98+
``model_validate()``, YAML/JSON deserialization, and ``__init__`` kwargs.
99+
100+
Pattern:
101+
1. Keep ``AliasChoices("new_name", "old_name")`` on the new field so
102+
old data still deserializes without a hard error.
103+
2. Add this ``model_validator(mode="before")`` to emit
104+
``logger.warning(...)`` for each deprecated key present in the raw
105+
input. The logger message appears in server logs where operators
106+
can see it, unlike ``warnings.warn`` which is filtered in tests and
107+
production by default.
108+
3. Mark the new field with ``Field(..., deprecated="...")`` for
109+
OpenAPI/JSON-schema visibility (requires Pydantic >= 2.7).
110+
4. Keep ``warnings.warn(DeprecationWarning)`` in ``__init__`` as a
111+
developer/test signal (visible via ``python -W`` or
112+
``pytest.warns``).
113+
"""
114+
if not isinstance(data, dict):
115+
return data
116+
if "instance_type" in data and "machine_type" not in data:
117+
logger.warning(
118+
"Template field 'instance_type' is deprecated; use 'machine_type' instead."
119+
)
120+
if "instance_profile" in data and "machine_role" not in data:
121+
logger.warning(
122+
"Template field 'instance_profile' is deprecated; use 'machine_role' instead."
123+
)
124+
return data
125+
85126
def __init__(self, **data: Any) -> None:
86127
"""Initialize template with default values and validation.
87128
@@ -91,30 +132,35 @@ def __init__(self, **data: Any) -> None:
91132
Note:
92133
Sets default name from template_id if not provided.
93134
Sets default timestamps if not provided.
94-
The deprecated ``instance_type`` keyword argument is accepted and
95-
silently promoted to ``machine_type``; a DeprecationWarning is
96-
emitted to encourage callers to update.
135+
The deprecated ``instance_type`` kwarg is accepted and mapped to
136+
``machine_type`` by Pydantic's AliasChoices; a DeprecationWarning
137+
is emitted here as a developer-visible signal (pytest.warns / -W),
138+
while the operator-visible logger.warning is emitted by the
139+
``_warn_deprecated_field_names`` model_validator above.
140+
141+
IMPORTANT: do NOT pop the deprecated key here — popping before
142+
calling ``super().__init__`` would hide the key from the
143+
model_validator(mode="before"), preventing the logger.warning.
144+
AliasChoices handles the field mapping after model_validator fires.
97145
"""
98-
# Promote deprecated instance_type kwarg → machine_type.
99-
# validation_alias handles model_validate() paths; __init__ kwargs need
100-
# explicit handling here because Pydantic v2 validation_alias is not
101-
# applied to __init__ keyword arguments.
146+
# Emit developer-facing DeprecationWarning for deprecated kwarg names.
147+
# Do NOT pop the keys — leave them in data so that model_validator
148+
# (mode="before") can see them and emit the operator-visible logger.warning.
149+
# AliasChoices will map instance_type → machine_type and
150+
# instance_profile → machine_role during Pydantic's validation pass.
102151
if "instance_type" in data and "machine_type" not in data:
103152
warnings.warn(
104153
"Template field 'instance_type' is deprecated; use 'machine_type' instead.",
105154
DeprecationWarning,
106155
stacklevel=2,
107156
)
108-
data["machine_type"] = data.pop("instance_type")
109157

110-
# Promote deprecated instance_profile kwarg → machine_role.
111158
if "instance_profile" in data and "machine_role" not in data:
112159
warnings.warn(
113160
"Template field 'instance_profile' is deprecated; use 'machine_role' instead.",
114161
DeprecationWarning,
115162
stacklevel=2,
116163
)
117-
data["machine_role"] = data.pop("instance_profile")
118164

119165
# Set default name if not provided
120166
if "name" not in data and "template_id" in data:

tests/unit/application/commands/test_template_command_handlers.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Unit tests for template command handlers — TDD for store-unification fix."""
22

3+
import logging
34
from unittest.mock import AsyncMock, Mock
45

56
import pytest
@@ -238,3 +239,86 @@ async def test_update_validation_errors_sets_updated_false() -> None:
238239
assert command.updated is False
239240
assert command.validation_errors == ["invalid field"]
240241
manager.save_template.assert_not_called()
242+
243+
244+
# ---------------------------------------------------------------------------
245+
# Deprecation warning tests — application-layer handler boundary
246+
# ---------------------------------------------------------------------------
247+
248+
249+
@pytest.mark.asyncio
250+
async def test_create_handler_warns_on_instance_type(caplog) -> None:
251+
"""CreateTemplateHandler emits logger.warning when instance_type is used."""
252+
manager = _make_manager(existing=None)
253+
port = _make_template_port(manager)
254+
handler = _make_create_handler(port)
255+
256+
command = CreateTemplateCommand(
257+
template_id="tpl-dep-1",
258+
provider_api="EC2Fleet",
259+
image_id="ami-000",
260+
instance_type="m5.xlarge",
261+
)
262+
263+
with caplog.at_level(logging.WARNING, logger="orb.application.commands.template_handlers"):
264+
await handler.handle(command)
265+
266+
assert command.created is True
267+
assert any(
268+
"instance_type" in r.message and "deprecated" in r.message for r in caplog.records
269+
), f"Expected deprecation log for instance_type; got: {[r.message for r in caplog.records]}"
270+
271+
# Value is mapped into machine_types on the saved DTO
272+
saved = manager.save_template.call_args[0][0]
273+
assert saved.machine_types == {"m5.xlarge": 1}
274+
275+
276+
@pytest.mark.asyncio
277+
async def test_update_handler_warns_on_instance_type(caplog) -> None:
278+
"""UpdateTemplateHandler emits logger.warning when instance_type is used."""
279+
existing = _make_dto()
280+
manager = _make_manager(existing=existing)
281+
port = _make_template_port(manager)
282+
handler = _make_update_handler(port)
283+
284+
command = UpdateTemplateCommand(
285+
template_id="tpl-dep-2",
286+
instance_type="c5.2xlarge",
287+
)
288+
289+
with caplog.at_level(logging.WARNING, logger="orb.application.commands.template_handlers"):
290+
await handler.handle(command)
291+
292+
assert command.updated is True
293+
assert any(
294+
"instance_type" in r.message and "deprecated" in r.message for r in caplog.records
295+
), f"Expected deprecation log for instance_type; got: {[r.message for r in caplog.records]}"
296+
297+
# Value is mapped into machine_types on the saved DTO
298+
saved = manager.save_template.call_args[0][0]
299+
assert saved.machine_types == {"c5.2xlarge": 1}
300+
301+
302+
@pytest.mark.asyncio
303+
async def test_create_handler_no_warn_without_instance_type(caplog) -> None:
304+
"""CreateTemplateHandler does NOT emit a deprecation log when machine_type is used."""
305+
manager = _make_manager(existing=None)
306+
port = _make_template_port(manager)
307+
handler = _make_create_handler(port)
308+
309+
command = CreateTemplateCommand(
310+
template_id="tpl-nodep-1",
311+
provider_api="EC2Fleet",
312+
image_id="ami-000",
313+
machine_type="t3.medium",
314+
)
315+
316+
with caplog.at_level(logging.WARNING, logger="orb.application.commands.template_handlers"):
317+
await handler.handle(command)
318+
319+
deprecation_records = [
320+
r for r in caplog.records if "instance_type" in r.message and "deprecated" in r.message
321+
]
322+
assert not deprecation_records, (
323+
f"Unexpected deprecation log when using machine_type: {[r.message for r in deprecation_records]}"
324+
)

tests/unit/domain/test_template_aggregate.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,107 @@ def test_old_name_instance_profile_not_readable_as_attribute(self):
549549
)
550550

551551

552+
@pytest.mark.unit
553+
class TestTemplateDeprecatedAliasesLoggerWarning:
554+
"""Tests that deprecated field names emit logger.warning on ALL deserialization paths.
555+
556+
AliasChoices silently accepts old keys via model_validate — these tests
557+
assert that operators see a warning in server logs even when using the
558+
YAML/JSON deserialization path (not just the Python-kwarg path).
559+
"""
560+
561+
def test_model_validate_instance_type_emits_logger_warning(self, caplog):
562+
"""model_validate with deprecated 'instance_type' emits a logger.warning."""
563+
import logging
564+
565+
with caplog.at_level(logging.WARNING, logger="orb.domain.template.template_aggregate"):
566+
t = Template.model_validate({"template_id": "lw-1", "instance_type": "m5.large"})
567+
assert t.machine_type == "m5.large"
568+
assert any(
569+
"instance_type" in r.message and "deprecated" in r.message for r in caplog.records
570+
), f"Expected deprecation log for instance_type; got: {[r.message for r in caplog.records]}"
571+
572+
def test_model_validate_instance_profile_emits_logger_warning(self, caplog):
573+
"""model_validate with deprecated 'instance_profile' emits a logger.warning."""
574+
import logging
575+
576+
with caplog.at_level(logging.WARNING, logger="orb.domain.template.template_aggregate"):
577+
t = Template.model_validate(
578+
{
579+
"template_id": "lw-2",
580+
"instance_profile": "arn:aws:iam::123456789012:instance-profile/legacy",
581+
}
582+
)
583+
assert t.machine_role == "arn:aws:iam::123456789012:instance-profile/legacy"
584+
assert any(
585+
"instance_profile" in r.message and "deprecated" in r.message for r in caplog.records
586+
), (
587+
f"Expected deprecation log for instance_profile; got: {[r.message for r in caplog.records]}"
588+
)
589+
590+
def test_kwarg_instance_type_emits_logger_warning(self, caplog):
591+
"""Template(instance_type=...) kwarg path also emits logger.warning (via model_validator)."""
592+
import logging
593+
594+
with caplog.at_level(logging.WARNING, logger="orb.domain.template.template_aggregate"):
595+
with pytest.warns(DeprecationWarning):
596+
t = Template(template_id="lw-3", instance_type="c5.xlarge")
597+
assert t.machine_type == "c5.xlarge"
598+
assert any(
599+
"instance_type" in r.message and "deprecated" in r.message for r in caplog.records
600+
), (
601+
f"Expected deprecation log for instance_type kwarg; got: {[r.message for r in caplog.records]}"
602+
)
603+
604+
def test_kwarg_instance_profile_emits_logger_warning(self, caplog):
605+
"""Template(instance_profile=...) kwarg path also emits logger.warning (via model_validator)."""
606+
import logging
607+
608+
with caplog.at_level(logging.WARNING, logger="orb.domain.template.template_aggregate"):
609+
with pytest.warns(DeprecationWarning):
610+
t = Template(
611+
template_id="lw-4",
612+
instance_profile="arn:aws:iam::123456789012:instance-profile/worker",
613+
)
614+
assert t.machine_role == "arn:aws:iam::123456789012:instance-profile/worker"
615+
assert any(
616+
"instance_profile" in r.message and "deprecated" in r.message for r in caplog.records
617+
), (
618+
f"Expected deprecation log for instance_profile kwarg; got: {[r.message for r in caplog.records]}"
619+
)
620+
621+
def test_new_field_names_emit_no_logger_warning(self, caplog):
622+
"""Using machine_type / machine_role directly produces no deprecation log."""
623+
import logging
624+
625+
with caplog.at_level(logging.WARNING, logger="orb.domain.template.template_aggregate"):
626+
t = Template.model_validate(
627+
{
628+
"template_id": "lw-5",
629+
"machine_type": "t3.medium",
630+
"machine_role": "arn:aws:iam::123456789012:instance-profile/new",
631+
}
632+
)
633+
assert t.machine_type == "t3.medium"
634+
assert t.machine_role == "arn:aws:iam::123456789012:instance-profile/new"
635+
assert not caplog.records, (
636+
f"Expected no deprecation log with new field names; got: {[r.message for r in caplog.records]}"
637+
)
638+
639+
def test_model_validate_instance_type_warns_once(self, caplog):
640+
"""Exactly one warning is emitted per deprecated key per model_validate call."""
641+
import logging
642+
643+
with caplog.at_level(logging.WARNING, logger="orb.domain.template.template_aggregate"):
644+
Template.model_validate({"template_id": "lw-6", "instance_type": "r5.large"})
645+
instance_type_warnings = [
646+
r for r in caplog.records if "instance_type" in r.message and "deprecated" in r.message
647+
]
648+
assert len(instance_type_warnings) == 1, (
649+
f"Expected exactly 1 warning; got {len(instance_type_warnings)}"
650+
)
651+
652+
552653
@pytest.mark.unit
553654
class TestTemplateExceptions:
554655
"""Test cases for Template-specific exceptions."""

0 commit comments

Comments
 (0)