From dc0843f1898e23574a2f5377f44fb4a7279ccd83 Mon Sep 17 00:00:00 2001 From: Stepan Burlakov Date: Mon, 4 Sep 2023 11:13:54 +0300 Subject: [PATCH 1/3] add support for failed engine state --- src/firebolt/service/types.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/firebolt/service/types.py b/src/firebolt/service/types.py index 022bb6ca549..703cf3d5cb5 100644 --- a/src/firebolt/service/types.py +++ b/src/firebolt/service/types.py @@ -47,6 +47,7 @@ class EngineStatus(Enum): STOPPED = "Stopped" DROPPING = "Dropping" REPAIRING = "Repairing" + FAILED = "Failed" def __str__(self) -> str: return self.value From afc408119fd34c58788fcf35a4e6e47f04d88e83 Mon Sep 17 00:00:00 2001 From: Stepan Burlakov Date: Mon, 4 Sep 2023 11:16:17 +0300 Subject: [PATCH 2/3] support passing 0 as auto_stop --- src/firebolt/model/engine.py | 6 ++++-- src/firebolt/service/engine.py | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/firebolt/model/engine.py b/src/firebolt/model/engine.py index 5ed19c25f9e..6c3e7dc4aed 100644 --- a/src/firebolt/model/engine.py +++ b/src/firebolt/model/engine.py @@ -213,7 +213,9 @@ def update( parameters are set to None, old engine parameter values remain. """ - if not any((name, scale, spec, auto_stop, warmup, engine_type)): + if not any( + x is not None for x in (name, scale, spec, auto_stop, warmup, engine_type) + ): # Nothing to be updated return self @@ -231,7 +233,7 @@ def update( self.ALTER_PARAMETER_NAMES, (scale, spec, auto_stop, name, warmup, engine_type), ): - if value: + if value is not None: sql += f"{param} = ? " parameters.append(str(value)) diff --git a/src/firebolt/service/engine.py b/src/firebolt/service/engine.py index 9f23a3eabfb..56334b534ef 100644 --- a/src/firebolt/service/engine.py +++ b/src/firebolt/service/engine.py @@ -156,13 +156,15 @@ def create( ("" if fail_if_exists else self.IF_NOT_EXISTS_SQL), name ) parameters = [] - if any((region, engine_type, spec, scale, auto_stop, warmup)): + if any( + x is not None for x in (region, engine_type, spec, scale, auto_stop, warmup) + ): sql += self.CREATE_WITH_SQL for param, value in zip( self.CREATE_PARAMETER_NAMES, (region, engine_type, spec, scale, auto_stop, warmup), ): - if value: + if value is not None: sql += f"{param} = ? " parameters.append(str(value)) with self._connection.cursor() as c: From b6f5230921ee862b01a6bf6fdfdf2d0a269e4906 Mon Sep 17 00:00:00 2001 From: Stepan Burlakov Date: Mon, 4 Sep 2023 11:22:08 +0300 Subject: [PATCH 3/3] add unit tests --- tests/unit/service/conftest.py | 7 +++++++ tests/unit/service/test_engine.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/tests/unit/service/conftest.py b/tests/unit/service/conftest.py index e6d3f9cb588..fc40e176ff2 100644 --- a/tests/unit/service/conftest.py +++ b/tests/unit/service/conftest.py @@ -307,12 +307,18 @@ def updated_engine_type() -> EngineType: return EngineType.DATA_ANALYTICS +@fixture +def updated_auto_stop() -> EngineType: + return 0 + + @fixture def update_engine_callback( system_engine_no_db_query_url: str, mock_engine: Engine, updated_engine_scale: int, updated_engine_type: EngineType, + updated_auto_stop: int, ) -> Callable: def do_mock( request: httpx.Request = None, @@ -321,6 +327,7 @@ def do_mock( assert request.url == system_engine_no_db_query_url mock_engine.scale = updated_engine_scale mock_engine.type = updated_engine_type + mock_engine.auto_stop = updated_auto_stop return Response( status_code=httpx.codes.OK, json=empty_response, diff --git a/tests/unit/service/test_engine.py b/tests/unit/service/test_engine.py index 13db5e6947e..2fd6f987df1 100644 --- a/tests/unit/service/test_engine.py +++ b/tests/unit/service/test_engine.py @@ -149,3 +149,32 @@ def test_engine_update( assert mock_engine.scale == updated_engine_scale assert mock_engine.type == updated_engine_type + + mock_engine.update(scale=updated_engine_scale, engine_type=updated_engine_type) + + assert mock_engine.scale == updated_engine_scale + assert mock_engine.type == updated_engine_type + + +def test_engine_update_auto_stop_zero( + httpx_mock: HTTPXMock, + resource_manager: ResourceManager, + instance_type_callback: Callable, + instance_type_url: str, + mock_engine: Engine, + get_engine_callback: Callable, + update_engine_callback: Callable, + system_engine_no_db_query_url: str, + updated_auto_stop: int, +): + httpx_mock.add_callback(instance_type_callback, url=instance_type_url) + httpx_mock.add_callback(get_engine_callback, url=system_engine_no_db_query_url) + httpx_mock.add_callback(update_engine_callback, url=system_engine_no_db_query_url) + httpx_mock.add_callback(get_engine_callback, url=system_engine_no_db_query_url) + + mock_engine.auto_stop = updated_auto_stop + 100 + # auto_stop = 0 is not considered an empty parameter value + mock_engine._service = resource_manager.engines + mock_engine.update(auto_stop=0) + + assert mock_engine.auto_stop == updated_auto_stop