From e7f53566f3aadd4815e50544a9eab8b68df8fa1b Mon Sep 17 00:00:00 2001 From: RotemFB Date: Thu, 10 Aug 2023 12:50:46 +0300 Subject: [PATCH 1/4] fixed delete on busy engine --- src/firebolt/model/database.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/firebolt/model/database.py b/src/firebolt/model/database.py index 118db61bb8c..a02ac4ca943 100644 --- a/src/firebolt/model/database.py +++ b/src/firebolt/model/database.py @@ -88,7 +88,7 @@ def delete(self) -> None: """ for engine in self.get_attached_engines(): - if engine.current_status not in { + if engine.current_status in { EngineStatus.STARTING, EngineStatus.STOPPING, }: From 66b8c3736a36351eb8880176a502c17777da3853 Mon Sep 17 00:00:00 2001 From: RotemFB Date: Wed, 6 Sep 2023 12:04:16 +0300 Subject: [PATCH 2/4] added a unit test for delete database --- tests/unit/service/conftest.py | 26 ++++++++++++++++++++++++++ tests/unit/service/test_database.py | 23 +++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/tests/unit/service/conftest.py b/tests/unit/service/conftest.py index a639376929c..1b64877bcc6 100644 --- a/tests/unit/service/conftest.py +++ b/tests/unit/service/conftest.py @@ -13,6 +13,7 @@ from firebolt.model.instance_type import InstanceType from firebolt.service.manager import ResourceManager from firebolt.service.types import EngineStatus, EngineType, WarmupMethod +from firebolt.utils.exception import DatabaseNotFoundError from firebolt.utils.urls import ACCOUNT_INSTANCE_TYPES_URL from tests.unit.util import list_to_paginated_response @@ -343,6 +344,31 @@ def do_mock( return do_mock +@fixture +def database_delete_callback() -> Callable: + def do_mock( + request: httpx.Request = None, + **kwargs, + ) -> Response: + return Response( + status_code=httpx.codes.OK, + json=empty_response, + ) + + return do_mock + +@fixture +def get_database_not_found_callback(mock_database: Database) -> Callable: + def do_mock( + request: httpx.Request = None, + **kwargs, + ) -> Response: + return Response( + status_code=httpx.codes.OK, + json=empty_response, + ) + + return do_mock @fixture def database_get_callback(mock_database) -> Callable: diff --git a/tests/unit/service/test_database.py b/tests/unit/service/test_database.py index df7038440ec..da35fae16d7 100644 --- a/tests/unit/service/test_database.py +++ b/tests/unit/service/test_database.py @@ -1,10 +1,12 @@ from typing import Callable +from pytest import raises from pytest_httpx import HTTPXMock from firebolt.model.database import Database from firebolt.model.engine import Engine from firebolt.service.manager import ResourceManager +from firebolt.utils.exception import DatabaseNotFoundError def test_database_create( @@ -90,3 +92,24 @@ def test_database_update( database = mock_database.update(description="new description") assert database.description == "new description" + +def test_database_delete( + httpx_mock: HTTPXMock, + resource_manager: ResourceManager, + database_delete_callback: Callable, + get_database_not_found_callback: Callable, + system_engine_no_db_query_url: str, + mock_database: Database, +): + httpx_mock.add_callback( + database_delete_callback, url=system_engine_no_db_query_url, method="POST" + ) + httpx_mock.add_callback( + get_database_not_found_callback, url=system_engine_no_db_query_url, method="POST" + ) + + mock_database._service = resource_manager.databases + mock_database.delete() + + with raises(DatabaseNotFoundError): + resource_manager.databases.get("invalid name") \ No newline at end of file From d63de83365c2b93483ad4edf79cf496731dbd9c6 Mon Sep 17 00:00:00 2001 From: RotemFB Date: Wed, 6 Sep 2023 12:57:32 +0300 Subject: [PATCH 3/4] pre commit modifications --- tests/unit/service/conftest.py | 4 +++- tests/unit/service/test_database.py | 11 +++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/unit/service/conftest.py b/tests/unit/service/conftest.py index 75a2bf4b85f..254955ff271 100644 --- a/tests/unit/service/conftest.py +++ b/tests/unit/service/conftest.py @@ -13,7 +13,6 @@ from firebolt.model.instance_type import InstanceType from firebolt.service.manager import ResourceManager from firebolt.service.types import EngineStatus, EngineType, WarmupMethod -from firebolt.utils.exception import DatabaseNotFoundError from firebolt.utils.urls import ACCOUNT_INSTANCE_TYPES_URL from tests.unit.util import list_to_paginated_response @@ -356,6 +355,7 @@ def do_mock( return do_mock + @fixture def database_delete_callback() -> Callable: def do_mock( @@ -369,6 +369,7 @@ def do_mock( return do_mock + @fixture def get_database_not_found_callback(mock_database: Database) -> Callable: def do_mock( @@ -382,6 +383,7 @@ def do_mock( return do_mock + @fixture def database_get_callback(mock_database) -> Callable: return get_objects_from_db_callback([mock_database]) diff --git a/tests/unit/service/test_database.py b/tests/unit/service/test_database.py index da35fae16d7..9c318c5df14 100644 --- a/tests/unit/service/test_database.py +++ b/tests/unit/service/test_database.py @@ -93,6 +93,7 @@ def test_database_update( assert database.description == "new description" + def test_database_delete( httpx_mock: HTTPXMock, resource_manager: ResourceManager, @@ -105,11 +106,13 @@ def test_database_delete( database_delete_callback, url=system_engine_no_db_query_url, method="POST" ) httpx_mock.add_callback( - get_database_not_found_callback, url=system_engine_no_db_query_url, method="POST" + get_database_not_found_callback, + url=system_engine_no_db_query_url, + method="POST", ) - + mock_database._service = resource_manager.databases mock_database.delete() - + with raises(DatabaseNotFoundError): - resource_manager.databases.get("invalid name") \ No newline at end of file + resource_manager.databases.get("invalid name") From 21374ee90578b31c2523340dc9f87c1daa16435e Mon Sep 17 00:00:00 2001 From: RotemFB Date: Tue, 12 Sep 2023 13:34:02 +0300 Subject: [PATCH 4/4] finished the unit test --- tests/unit/service/conftest.py | 53 ++++++++++++++--------------- tests/unit/service/test_database.py | 24 ++++++------- 2 files changed, 35 insertions(+), 42 deletions(-) diff --git a/tests/unit/service/conftest.py b/tests/unit/service/conftest.py index 254955ff271..e6d3f9cb588 100644 --- a/tests/unit/service/conftest.py +++ b/tests/unit/service/conftest.py @@ -40,6 +40,26 @@ def mock_engine(region: str, server: str, instance_type_1: InstanceType) -> Engi ) +@fixture +def mock_engine_stopping( + region: str, server: str, instance_type_1: InstanceType +) -> Engine: + return Engine( + name="engine_1", + region=region, + spec=instance_type_1, + scale=2, + current_status=EngineStatus.STOPPING, + version="", + endpoint=server, + warmup=WarmupMethod.MINIMAL, + auto_stop=7200, + type=EngineType.GENERAL_PURPOSE, + _database_name="database", + _service=None, + ) + + @fixture def instance_type_1() -> InstanceType: return InstanceType( @@ -243,6 +263,11 @@ def get_engine_callback(mock_engine: Engine) -> Callable: return get_objects_from_db_callback([mock_engine]) +@fixture +def get_engine_callback_stopping(mock_engine_stopping: Engine) -> Callable: + return get_objects_from_db_callback([mock_engine_stopping]) + + @fixture def get_engine_not_found_callback(mock_engine: Engine) -> Callable: def do_mock( @@ -356,34 +381,6 @@ def do_mock( return do_mock -@fixture -def database_delete_callback() -> Callable: - def do_mock( - request: httpx.Request = None, - **kwargs, - ) -> Response: - return Response( - status_code=httpx.codes.OK, - json=empty_response, - ) - - return do_mock - - -@fixture -def get_database_not_found_callback(mock_database: Database) -> Callable: - def do_mock( - request: httpx.Request = None, - **kwargs, - ) -> Response: - return Response( - status_code=httpx.codes.OK, - json=empty_response, - ) - - return do_mock - - @fixture def database_get_callback(mock_database) -> Callable: return get_objects_from_db_callback([mock_database]) diff --git a/tests/unit/service/test_database.py b/tests/unit/service/test_database.py index 9c318c5df14..f5832dcbe42 100644 --- a/tests/unit/service/test_database.py +++ b/tests/unit/service/test_database.py @@ -6,7 +6,7 @@ from firebolt.model.database import Database from firebolt.model.engine import Engine from firebolt.service.manager import ResourceManager -from firebolt.utils.exception import DatabaseNotFoundError +from firebolt.utils.exception import AttachedEngineInUseError def test_database_create( @@ -94,25 +94,21 @@ def test_database_update( assert database.description == "new description" -def test_database_delete( +def test_database_delete_busy_engine( httpx_mock: HTTPXMock, resource_manager: ResourceManager, - database_delete_callback: Callable, - get_database_not_found_callback: Callable, system_engine_no_db_query_url: str, + get_engine_callback_stopping: Engine, mock_database: Database, + instance_type_callback: Callable, + instance_type_url: str, ): + httpx_mock.add_callback(instance_type_callback, url=instance_type_url) httpx_mock.add_callback( - database_delete_callback, url=system_engine_no_db_query_url, method="POST" - ) - httpx_mock.add_callback( - get_database_not_found_callback, - url=system_engine_no_db_query_url, - method="POST", + get_engine_callback_stopping, url=system_engine_no_db_query_url ) - mock_database._service = resource_manager.databases - mock_database.delete() + mock_database._service = resource_manager.engines - with raises(DatabaseNotFoundError): - resource_manager.databases.get("invalid name") + with raises(AttachedEngineInUseError): + mock_database.delete()