Skip to content

Commit 4808e81

Browse files
authored
fix(FIR-50579): Avoid breaking on unknown engine configs (#473)
1 parent d3af11d commit 4808e81

File tree

5 files changed

+76
-4
lines changed

5 files changed

+76
-4
lines changed

src/firebolt/model/V2/engine.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ class Engine(FireboltBaseModel):
5656

5757
def __post_init__(self) -> None:
5858
# Specs are just strings for accounts v2
59-
if isinstance(self.spec, str) and self.spec:
59+
if isinstance(self.spec, str):
6060
# Resolve engine specification
6161
self.spec = InstanceType(self.spec)
62-
if isinstance(self.current_status, str) and self.current_status:
62+
if isinstance(self.current_status, str):
6363
# Resolve engine status
6464
self.current_status = EngineStatus(self.current_status)
6565

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
from enum import Enum
2+
from typing import Any
23

34

45
class InstanceType(Enum):
56
S = "S"
67
M = "M"
78
L = "L"
89
XL = "XL"
10+
UNKNOWN = "UNKNOWN" # instance type could not be determined
11+
12+
@classmethod
13+
def _missing_(cls, value: Any) -> "InstanceType":
14+
return cls.UNKNOWN

src/firebolt/service/V2/types.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from enum import Enum
2+
from typing import Any
23

34

45
class EngineStatus(Enum):
@@ -15,6 +16,13 @@ class EngineStatus(Enum):
1516
REPAIRING = "REPAIRING"
1617
FAILED = "FAILED"
1718
DELETING = "DELETING"
19+
RESIZING = "RESIZING"
20+
DRAINING = "DRAINING"
21+
UNKNOWN = "UNKNOWN" # status could not be determined
22+
23+
@classmethod
24+
def _missing_(cls, value: Any) -> "EngineStatus":
25+
return cls.UNKNOWN
1826

1927
def __str__(self) -> str:
2028
return self.value

tests/unit/service/test_engine.py

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
from typing import Callable
1+
from typing import Callable, Union
22

33
from httpx import Request
44
from pytest import mark, raises
55
from pytest_httpx import HTTPXMock
66

77
from firebolt.model.V2.database import Database
88
from firebolt.model.V2.engine import Engine, EngineStatus
9+
from firebolt.model.V2.instance_type import InstanceType
910
from firebolt.service.manager import ResourceManager
1011
from firebolt.utils.exception import EngineNotFoundError
1112
from tests.unit.response import Response
@@ -219,3 +220,59 @@ def test_engine_new_status(
219220
engine = resource_manager.engines.get_by_name(mock_engine.name)
220221

221222
assert engine.current_status == expected_status
223+
224+
225+
@mark.parametrize(
226+
"spec_input, expected_spec, status_input, expected_status",
227+
[
228+
# Test all valid InstanceType values
229+
("S", InstanceType.S, "RUNNING", EngineStatus.RUNNING),
230+
("M", InstanceType.M, "STOPPED", EngineStatus.STOPPED),
231+
("L", InstanceType.L, "STARTING", EngineStatus.STARTING),
232+
("XL", InstanceType.XL, "STOPPING", EngineStatus.STOPPING),
233+
# Test InstanceType enum values directly
234+
(InstanceType.S, InstanceType.S, "FAILED", EngineStatus.FAILED),
235+
(InstanceType.M, InstanceType.M, "REPAIRING", EngineStatus.REPAIRING),
236+
# Test unknown/invalid values that should default to UNKNOWN
237+
("INVALID_TYPE", InstanceType.UNKNOWN, "INVALID_STATUS", EngineStatus.UNKNOWN),
238+
("XXL", InstanceType.UNKNOWN, "WEIRD_STATE", EngineStatus.UNKNOWN),
239+
# Test empty strings that should default to UNKNOWN
240+
("", InstanceType.UNKNOWN, "", EngineStatus.UNKNOWN),
241+
# Test all valid EngineStatus values with M instance type
242+
("M", InstanceType.M, "STARTED", EngineStatus.STARTED),
243+
("M", InstanceType.M, "DROPPING", EngineStatus.DROPPING),
244+
("M", InstanceType.M, "DELETING", EngineStatus.DELETING),
245+
("M", InstanceType.M, "RESIZING", EngineStatus.RESIZING),
246+
("M", InstanceType.M, "DRAINING", EngineStatus.DRAINING),
247+
],
248+
)
249+
def test_engine_instantiation_with_different_configurations(
250+
spec_input: Union[str, InstanceType],
251+
expected_spec: InstanceType,
252+
status_input: str,
253+
expected_status: EngineStatus,
254+
) -> None:
255+
"""
256+
Test that Engine model correctly handles different instance types and statuses,
257+
including unknown values and empty strings that should default to UNKNOWN.
258+
"""
259+
engine = Engine(
260+
name="test_engine",
261+
region="us-east-1",
262+
spec=spec_input,
263+
scale=2,
264+
current_status=status_input,
265+
version="1.0",
266+
endpoint="https://test.endpoint.com",
267+
warmup="",
268+
auto_stop=3600,
269+
type="general_purpose",
270+
_database_name="test_db",
271+
_service=None,
272+
)
273+
274+
assert engine.spec == expected_spec
275+
assert engine.current_status == expected_status
276+
assert engine.name == "test_engine"
277+
assert engine.region == "us-east-1"
278+
assert engine.scale == 2

tests/unit/service/test_instance_type.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ def test_instance_types(resource_manager: ResourceManager):
88
InstanceType.M,
99
InstanceType.L,
1010
InstanceType.XL,
11+
InstanceType.UNKNOWN,
1112
]
1213

1314
assert resource_manager.instance_types.get("XL") == InstanceType.XL
14-
assert resource_manager.instance_types.get("XS") is None
15+
assert resource_manager.instance_types.get("XS") == InstanceType.UNKNOWN
1516

1617
assert resource_manager.instance_types.cheapest_instance() == InstanceType.S

0 commit comments

Comments
 (0)