Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ project_urls =
packages = find:
install_requires =
aiorwlock==1.1.0
anyio<4.0.0
appdirs>=1.4.4
appdirs-stubs>=0.1.0
async-generator>=1.10
Expand Down
7 changes: 5 additions & 2 deletions src/firebolt/model/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class Engine(FireboltBaseModel):
"AUTO_STOP",
"RENAME_TO",
"WARMUP",
"ENGINE_TYPE",
)
DROP_SQL: ClassVar[str] = "DROP ENGINE {}"

Expand Down Expand Up @@ -201,6 +202,7 @@ def stop(self) -> Engine:
def update(
self,
name: Optional[str] = None,
engine_type: Union[EngineType, str, None] = None,
scale: Optional[int] = None,
spec: Union[InstanceType, str, None] = None,
auto_stop: Optional[int] = None,
Expand All @@ -211,7 +213,7 @@ def update(
parameters are set to None, old engine parameter values remain.
"""

if not any((name, scale, spec, auto_stop, warmup)):
if not any((name, scale, spec, auto_stop, warmup, engine_type)):
# Nothing to be updated
return self

Expand All @@ -226,7 +228,8 @@ def update(
sql = self.ALTER_PREFIX_SQL.format(self.name)
parameters = []
for param, value in zip(
self.ALTER_PARAMETER_NAMES, (scale, spec, auto_stop, name, warmup)
self.ALTER_PARAMETER_NAMES,
(scale, spec, auto_stop, name, warmup, engine_type),
):
if value:
sql += f"{param} = ? "
Expand Down
26 changes: 19 additions & 7 deletions tests/integration/resource_manager/test_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from firebolt.client.auth import Auth
from firebolt.service.manager import ResourceManager
from firebolt.service.types import EngineStatus
from firebolt.service.types import EngineStatus, EngineType, WarmupMethod


def make_engine_name(database_name: str, suffix: str) -> str:
Expand Down Expand Up @@ -41,11 +41,11 @@ def test_create_start_stop_engine(

ParamValue = namedtuple("ParamValue", "set expected")
ENGINE_UPDATE_PARAMS = {
# commented parameters are not available yet
# "scale": ParamValue(23, 23),
# "spec": ParamValue("B1", "B1"),
"scale": ParamValue(3, 3),
"spec": ParamValue("B1", "B1"),
"auto_stop": ParamValue(123, 7380),
# "warmup": ParamValue(WarmupMethod.PRELOAD_ALL_DATA, WarmupMethod.PRELOAD_ALL_DATA),
"warmup": ParamValue(WarmupMethod.PRELOAD_ALL_DATA, WarmupMethod.PRELOAD_ALL_DATA),
"engine_type": ParamValue(EngineType.DATA_ANALYTICS, EngineType.DATA_ANALYTICS),
}


Expand All @@ -69,7 +69,13 @@ def test_engine_update_single_parameter(
engine.update(**{param: value.set})

engine_new = rm.engines.get(name)
assert getattr(engine_new, param) == value.expected, f"Invalid {param} value"
if param == "spec":
current_value = engine_new.spec.name
elif param == "engine_type":
current_value = engine_new.type
else:
current_value = getattr(engine_new, param)
assert current_value == value.expected, f"Invalid {param} value"

engine.delete()

Expand Down Expand Up @@ -97,6 +103,12 @@ def test_engine_update_multiple_parameters(
engine_new = rm.engines.get(name)

for param, value in ENGINE_UPDATE_PARAMS.items():
assert getattr(engine_new, param) == value.expected, f"Invalid {param} value"
if param == "spec":
current_value = engine_new.spec.name
elif param == "engine_type":
current_value = engine_new.type
else:
current_value = getattr(engine_new, param)
assert current_value == value.expected, f"Invalid {param} value"

engine.delete()
14 changes: 13 additions & 1 deletion tests/unit/service/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,15 @@ def get_obj_field(obj, f):
value = getattr(obj, f.name)
if isinstance(value, (InstanceType, EngineStatus)):
return str(value)
if isinstance(value, (EngineType, WarmupMethod)):
if isinstance(value, WarmupMethod):
return " ".join(
map(lambda s: s.capitalize(), str(value).lower().split("_"))
)
if isinstance(value, EngineType):
return {
EngineType.GENERAL_PURPOSE: "General Purpose",
EngineType.DATA_ANALYTICS: "Analytics",
}[value]
return value

query_response = {
Expand Down Expand Up @@ -272,18 +277,25 @@ def updated_engine_scale() -> int:
return 10


@fixture
def updated_engine_type() -> EngineType:
return EngineType.DATA_ANALYTICS


@fixture
def update_engine_callback(
system_engine_no_db_query_url: str,
mock_engine: Engine,
updated_engine_scale: int,
updated_engine_type: EngineType,
) -> Callable:
def do_mock(
request: httpx.Request = None,
**kwargs,
) -> Response:
assert request.url == system_engine_no_db_query_url
mock_engine.scale = updated_engine_scale
mock_engine.type = updated_engine_type
return Response(
status_code=httpx.codes.OK,
json=empty_response,
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/service/test_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from firebolt.model.database import Database
from firebolt.model.engine import Engine
from firebolt.service.manager import ResourceManager
from firebolt.service.types import EngineType
from firebolt.utils.exception import (
EngineNotFoundError,
NoAttachedDatabaseError,
Expand Down Expand Up @@ -136,13 +137,15 @@ def test_engine_update(
update_engine_callback: Callable,
system_engine_no_db_query_url: str,
updated_engine_scale: int,
updated_engine_type: EngineType,
):
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._service = resource_manager.engines
mock_engine.update(scale=updated_engine_scale)
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