-
Notifications
You must be signed in to change notification settings - Fork 79
feat: [SNOW-2155083] spcs service: support auto-suspend-secs #2487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: [SNOW-2155083] spcs service: support auto-suspend-secs #2487
Conversation
Added entry for new auto_suspend_secs parameter functionality in SPCS service commands (deploy, set, unset) that allows configuring automatic service suspension after inactivity period.
d7faffb to
6d93b48
Compare
| max_instances = 3 | ||
| query_warehouse = "test_warehouse" | ||
| auto_resume = False | ||
| auto_suspend_secs = 600 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
| @patch(EXECUTE_QUERY) | ||
| def test_set_property_auto_suspend_secs_only(mock_execute_query): | ||
| service_name = "test_service" | ||
| auto_suspend_secs = 300 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
Added entry for new auto_suspend_secs parameter functionality in SPCS service commands (deploy, set, unset) that allows configuring automatic service suspension after inactivity period.
Pre-review checklist
Changes description
https://snowflakecomputing.atlassian.net/browse/SNOW-2155083