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 RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

## New additions
* Added global option `--decimal-precision` allowing setting arbitrary precision for Python's `Decimal` type.
* Added support for `auto_suspend_secs` parameter in SPCS service commands (`deploy`, `set`, `unset`) to configure automatic service suspension after inactivity period.

## Fixes and improvements
* Bumped `snowflake-connector-python==3.18.0`
Expand Down
20 changes: 19 additions & 1 deletion src/snowflake/cli/_plugins/spcs/services/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ def _service_name_callback(name: FQN) -> FQN:
help=_AUTO_RESUME_HELP,
)

_AUTO_SUSPEND_SECS_HELP = "Number of seconds of inactivity after which the service will be automatically suspended."
AutoSuspendSecsOption = OverrideableOption(
None,
"--auto-suspend-secs",
help=_AUTO_SUSPEND_SECS_HELP,
min=0,
)

_COMMENT_HELP = "Comment for the service."

add_object_command_aliases(
Expand Down Expand Up @@ -217,7 +225,7 @@ def deploy(
upgrade: bool = typer.Option(
False,
"--upgrade",
help="Updates the existing service. Can update min_instances, max_instances, query_warehouse, auto_resume, external_access_integrations and comment.",
help="Updates the existing service. Can update min_instances, max_instances, query_warehouse, auto_resume, auto_suspend_secs, external_access_integrations and comment.",
),
**options,
) -> CommandResult:
Expand All @@ -241,6 +249,7 @@ def deploy(
min_instances=service.min_instances,
max_instances=max_instances,
auto_resume=service.auto_resume,
auto_suspend_secs=service.auto_suspend_secs,
external_access_integrations=service.external_access_integrations,
query_warehouse=service.query_warehouse,
tags=service.tags,
Expand Down Expand Up @@ -529,6 +538,7 @@ def set_property(
max_instances: Optional[int] = MaxInstancesOption(show_default=False),
query_warehouse: Optional[str] = QueryWarehouseOption(show_default=False),
auto_resume: Optional[bool] = AutoResumeOption(default=None, show_default=False),
auto_suspend_secs: Optional[int] = AutoSuspendSecsOption(show_default=False),
external_access_integrations: Optional[List[str]] = typer.Option(
None,
"--eai-name",
Expand All @@ -546,6 +556,7 @@ def set_property(
max_instances=max_instances,
query_warehouse=query_warehouse,
auto_resume=auto_resume,
auto_suspend_secs=auto_suspend_secs,
external_access_integrations=external_access_integrations,
comment=comment,
)
Expand Down Expand Up @@ -576,6 +587,12 @@ def unset_property(
help=f"Reset the AUTO_RESUME property - {_AUTO_RESUME_HELP}",
show_default=False,
),
auto_suspend_secs: bool = AutoSuspendSecsOption(
default=False,
param_decls=["--auto-suspend-secs"],
help=f"Reset the AUTO_SUSPEND_SECS property - {_AUTO_SUSPEND_SECS_HELP}",
show_default=False,
),
comment: bool = CommentOption(
default=False,
help=f"Reset the COMMENT property - {_COMMENT_HELP}",
Expand All @@ -593,6 +610,7 @@ def unset_property(
max_instances=max_instances,
query_warehouse=query_warehouse,
auto_resume=auto_resume,
auto_suspend_secs=auto_suspend_secs,
comment=comment,
)
return SingleQueryResult(cursor)
12 changes: 12 additions & 0 deletions src/snowflake/cli/_plugins/spcs/services/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def deploy(
min_instances: int,
max_instances: int,
auto_resume: bool,
auto_suspend_secs: Optional[int],
external_access_integrations: Optional[List[str]],
query_warehouse: Optional[str],
tags: Optional[List[Tag]],
Expand All @@ -139,6 +140,7 @@ def deploy(
max_instances=max_instances,
query_warehouse=query_warehouse,
auto_resume=auto_resume,
auto_suspend_secs=auto_suspend_secs,
external_access_integrations=external_access_integrations,
comment=comment,
)
Expand All @@ -163,6 +165,9 @@ def deploy(
if max_instances:
query.append(f"MAX_INSTANCES = {max_instances}")

if auto_suspend_secs is not None:
query.append(f"AUTO_SUSPEND_SECS = {auto_suspend_secs}")

if query_warehouse:
query.append(f"QUERY_WAREHOUSE = {query_warehouse}")

Expand Down Expand Up @@ -532,6 +537,7 @@ def set_property(
max_instances: Optional[int],
query_warehouse: Optional[str],
auto_resume: Optional[bool],
auto_suspend_secs: Optional[int],
external_access_integrations: Optional[List[str]],
comment: Optional[str],
):
Expand All @@ -540,6 +546,7 @@ def set_property(
("max_instances", max_instances),
("query_warehouse", query_warehouse),
("auto_resume", auto_resume),
("auto_suspend_secs", auto_suspend_secs),
("external_access_integrations", external_access_integrations),
("comment", comment),
]
Expand All @@ -563,6 +570,9 @@ def set_property(
if auto_resume is not None:
query.append(f" auto_resume = {auto_resume}")

if auto_suspend_secs is not None:
query.append(f" auto_suspend_secs = {auto_suspend_secs}")

if external_access_integrations is not None:
external_access_integration_list = ",".join(
f"{e}" for e in external_access_integrations
Expand All @@ -583,13 +593,15 @@ def unset_property(
max_instances: bool,
query_warehouse: bool,
auto_resume: bool,
auto_suspend_secs: bool,
comment: bool,
):
property_pairs = [
("min_instances", min_instances),
("max_instances", max_instances),
("query_warehouse", query_warehouse),
("auto_resume", auto_resume),
("auto_suspend_secs", auto_suspend_secs),
("comment", comment),
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ class ServiceEntityModel(EntityModelBaseWithArtifacts, ExternalAccessBaseModel):
title="The service will automatically resume when a service function or ingress is called.",
default=True,
)
auto_suspend_secs: Optional[int] = Field(
title="Number of seconds of inactivity after which the service is automatically suspended.",
default=None,
ge=0,
)
query_warehouse: Optional[str] = Field(
title="Warehouse to use if a service container connects to Snowflake to execute a query without explicitly specifying a warehouse to use",
default=None,
Expand Down
43 changes: 27 additions & 16 deletions tests/__snapshots__/test_help_messages.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -16583,8 +16583,8 @@
+- Options --------------------------------------------------------------------+
| --upgrade Updates the existing service. Can update |
| min_instances, max_instances, query_warehouse, |
| auto_resume, external_access_integrations and |
| comment. |
| auto_resume, auto_suspend_secs, |
| external_access_integrations and comment. |
| --project -p TEXT Path where the Snowflake project is stored. |
| Defaults to the current working directory. |
| --env TEXT String in the format key=value. Overrides variables |
Expand Down Expand Up @@ -18335,6 +18335,13 @@
| service function |
| or ingress is |
| called. |
| --auto-suspend-s… INTEGER RANGE Number of |
| [x>=0] seconds of |
| inactivity after |
| which the |
| service will be |
| automatically |
| suspended. |
| --eai-name TEXT Identifies |
| external access |
| integrations |
Expand Down Expand Up @@ -18774,20 +18781,24 @@
| [required] |
+------------------------------------------------------------------------------+
+- Options --------------------------------------------------------------------+
| --min-instances Reset the MIN_INSTANCES property - Minimum |
| number of service instances to run. |
| --max-instances Reset the MAX_INSTANCES property - Maximum |
| number of service instances to run. |
| --query-warehouse Reset the QUERY_WAREHOUSE property - Warehouse |
| to use if a service container connects to |
| Snowflake to execute a query without explicitly |
| specifying a warehouse to use. |
| --auto-resume Reset the AUTO_RESUME property - The service |
| will automatically resume when a service |
| function or ingress is called. |
| --comment Reset the COMMENT property - Comment for the |
| service. |
| --help -h Show this message and exit. |
| --min-instances Reset the MIN_INSTANCES property - Minimum |
| number of service instances to run. |
| --max-instances Reset the MAX_INSTANCES property - Maximum |
| number of service instances to run. |
| --query-warehouse Reset the QUERY_WAREHOUSE property - |
| Warehouse to use if a service container |
| connects to Snowflake to execute a query |
| without explicitly specifying a warehouse to |
| use. |
| --auto-resume Reset the AUTO_RESUME property - The service |
| will automatically resume when a service |
| function or ingress is called. |
| --auto-suspend-secs Reset the AUTO_SUSPEND_SECS property - Number |
| of seconds of inactivity after which the |
| service will be automatically suspended. |
| --comment Reset the COMMENT property - Comment for the |
| service. |
| --help -h Show this message and exit. |
+------------------------------------------------------------------------------+
+- Connection configuration ---------------------------------------------------+
| --connection,--environment -c TEXT Name of the connection, as |
Expand Down
61 changes: 57 additions & 4 deletions tests/spcs/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,7 @@ def test_set_property(mock_execute_query):
max_instances = 3
query_warehouse = "test_warehouse"
auto_resume = False
auto_suspend_secs = 600
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could be defined instead of magic number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of those matter really, it's just an integer. This variable is local to this test only, I wouldn't see any point in extracting values of service_name, min_instances, max_instances etc either

external_access_integrations = [
"google_apis_access_integration",
"salesforce_api_access_integration",
Expand All @@ -1544,6 +1545,7 @@ def test_set_property(mock_execute_query):
max_instances=max_instances,
query_warehouse=query_warehouse,
auto_resume=auto_resume,
auto_suspend_secs=auto_suspend_secs,
external_access_integrations=external_access_integrations,
comment=comment,
)
Expand All @@ -1555,6 +1557,7 @@ def test_set_property(mock_execute_query):
f"max_instances = {max_instances}",
f"query_warehouse = {query_warehouse}",
f"auto_resume = {auto_resume}",
f"auto_suspend_secs = {auto_suspend_secs}",
f"external_access_integrations = ({eai_list})",
f"comment = {comment}",
]
Expand All @@ -1566,7 +1569,9 @@ def test_set_property(mock_execute_query):
def test_set_property_no_properties():
service_name = "test_service"
with pytest.raises(NoPropertiesProvidedError) as e:
ServiceManager().set_property(service_name, None, None, None, None, None, None)
ServiceManager().set_property(
service_name, None, None, None, None, None, None, None
)
assert (
e.value.message
== f"No properties specified for service '{service_name}'. Please provide at least one property to set."
Expand All @@ -1582,6 +1587,7 @@ def test_set_property_cli(mock_set, mock_statement_success, runner):
max_instances = 3
query_warehouse = "test_warehouse"
auto_resume = False
auto_suspend_secs = 600
external_access_integrations = [
"google_apis_access_integration",
"salesforce_api_access_integration",
Expand All @@ -1600,6 +1606,8 @@ def test_set_property_cli(mock_set, mock_statement_success, runner):
"--query-warehouse",
query_warehouse,
"--no-auto-resume",
"--auto-suspend-secs",
str(auto_suspend_secs),
"--eai-name",
"google_apis_access_integration",
"--eai-name",
Expand All @@ -1614,6 +1622,7 @@ def test_set_property_cli(mock_set, mock_statement_success, runner):
max_instances=max_instances,
query_warehouse=query_warehouse,
auto_resume=auto_resume,
auto_suspend_secs=auto_suspend_secs,
external_access_integrations=external_access_integrations,
comment=to_string_literal(comment),
)
Expand All @@ -1636,6 +1645,7 @@ def test_set_property_no_properties_cli(mock_set, runner):
max_instances=None,
query_warehouse=None,
auto_resume=None,
auto_suspend_secs=None,
external_access_integrations=None,
comment=None,
)
Expand All @@ -1646,16 +1656,20 @@ def test_unset_property(mock_execute_query):
service_name = "test_service"
cursor = Mock(spec=SnowflakeCursor)
mock_execute_query.return_value = cursor
result = ServiceManager().unset_property(service_name, True, True, True, True, True)
expected_query = "alter service test_service unset min_instances,max_instances,query_warehouse,auto_resume,comment"
result = ServiceManager().unset_property(
service_name, True, True, True, True, True, True
)
expected_query = "alter service test_service unset min_instances,max_instances,query_warehouse,auto_resume,auto_suspend_secs,comment"
mock_execute_query.assert_called_once_with(expected_query)
assert result == cursor


def test_unset_property_no_properties():
service_name = "test_service"
with pytest.raises(NoPropertiesProvidedError) as e:
ServiceManager().unset_property(service_name, False, False, False, False, False)
ServiceManager().unset_property(
service_name, False, False, False, False, False, False
)
assert (
e.value.message
== f"No properties specified for service '{service_name}'. Please provide at least one property to reset to its default value."
Expand All @@ -1677,6 +1691,7 @@ def test_unset_property_cli(mock_unset, mock_statement_success, runner):
"--max-instances",
"--query-warehouse",
"--auto-resume",
"--auto-suspend-secs",
"--comment",
]
)
Expand All @@ -1686,6 +1701,7 @@ def test_unset_property_cli(mock_unset, mock_statement_success, runner):
max_instances=True,
query_warehouse=True,
auto_resume=True,
auto_suspend_secs=True,
comment=True,
)
assert result.exit_code == 0, result.output
Expand All @@ -1707,10 +1723,47 @@ def test_unset_property_no_properties_cli(mock_unset, runner):
max_instances=False,
query_warehouse=False,
auto_resume=False,
auto_suspend_secs=False,
comment=False,
)


@patch(EXECUTE_QUERY)
def test_set_property_auto_suspend_secs_only(mock_execute_query):
service_name = "test_service"
auto_suspend_secs = 300
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is another magic value. Why not 600 like earlier?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above, the exact value doesn't really matter, it just needs to be some positive integer

cursor = Mock(spec=SnowflakeCursor)
mock_execute_query.return_value = cursor
result = ServiceManager().set_property(
service_name=service_name,
min_instances=None,
max_instances=None,
query_warehouse=None,
auto_resume=None,
auto_suspend_secs=auto_suspend_secs,
external_access_integrations=None,
comment=None,
)
expected_query = (
f"alter service {service_name} set\nauto_suspend_secs = {auto_suspend_secs}"
)
mock_execute_query.assert_called_once_with(expected_query)
assert result == cursor


@patch(EXECUTE_QUERY)
def test_unset_property_auto_suspend_secs_only(mock_execute_query):
service_name = "test_service"
cursor = Mock(spec=SnowflakeCursor)
mock_execute_query.return_value = cursor
result = ServiceManager().unset_property(
service_name, False, False, False, False, True, False
)
expected_query = f"alter service {service_name} unset auto_suspend_secs"
mock_execute_query.assert_called_once_with(expected_query)
assert result == cursor


def test_unset_property_with_args(runner):
service_name = "test_service"
result = runner.invoke(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
definition_version: "2"

entities:
service:
type: service
identifier:
name:
stage:
compute_pool: snowcli_compute_pool
spec_file: spec.yml
min_instances: 1
max_instances: 1
query_warehouse: xsmall
auto_suspend_secs: 600
comment: "Test service without public endpoint"
artifacts:
- spec.yml
Loading