Repository navigation
feat(resource_monitor): manage triggers, several notify users, and non-ACCOUNTADMIN owners - #86
Open
GClunies wants to merge 1 commit into
Conversation
…n-ACCOUNTADMIN owners
A live resource monitor could not be declared and planned to zero changes:
- The class had no field for TRIGGERS, so notify and suspend thresholds
could not be declared or read back.
- The fetch returned the SHOW notify_users string ("A, B") instead of a
list, so a monitor with several notify users failed to plan.
- The spec refused any owner except ACCOUNTADMIN, although ACCOUNTADMIN can
hand a monitor to another role, and after that only that role can alter
or drop it.
Add a `triggers` field (threshold + NOTIFY / SUSPEND / SUSPEND_IMMEDIATE)
rendered and parsed by TriggersProp, read it back from notify_at /
suspend_at / suspend_immediately_at, and sort triggers and notify users so
order never diffs. Updates emit SET props followed by the TRIGGERS clause.
Create runs as ACCOUNTADMIN and transfers to a declared owner; update and
drop run as the owning role.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Snowcap can now manage a real resource monitor: its triggers, a list of notify users, and an owner other than
ACCOUNTADMIN. A live monitor declared in YAML plans to zero changes, and changes to it apply.ResourceMonitorhad no field forTRIGGERS. Notify and suspend thresholds could not be declared or read back.triggersfield: a list of{threshold, action}with actionNOTIFY,SUSPEND, orSUSPEND_IMMEDIATE. The fetch reads it back fromnotify_at,suspend_at, andsuspend_immediately_at.SHOWstring"A, B", so the plan rejected the monitor it read.ResourceMonitors can only be created by ACCOUNTADMINfor any other owner.ACCOUNTADMINand transfers ownership to the declared owner. Update and drop run as the owning role.Why
We tested each Snowflake behavior against a live account before we wrote the code.
ALTER RESOURCE MONITOR X SET CREDIT_QUOTA = 7 TRIGGERS ON ...TRIGGERSreplaces every trigger.update_resource_monitorrendersSETprops, then the fullTRIGGERSclause.ALTER RESOURCE MONITOR X SET TRIGGERS ON ...TRIGGERSis not aSETproperty.update__defaultcannot render it, so the resource has its own update function.UNSET TRIGGERS, a bareTRIGGERS,SET TRIGGERS = (),NOTRIGGERStriggers: []raisesValueError. Omittriggersto leave them unmanaged.TRIGGERS ON 75.5 PERCENT DO NOTIFYValueError.SHOW RESOURCE MONITORSnotify_at'50%,75%'. One production account reports'100%,50%,75%,90%', which is not numeric order.SHOW RESOURCE MONITORSnotify_users''or'A, B, C'.ALTERorDROPon a monitor owned by another role, run asACCOUNTADMINGRANT OWNERSHIP ON RESOURCE MONITOR X TO ROLE RasACCOUNTADMINACCOUNTADMINwith an ownership transfer when the owner differs.Tested
TestResourceMonitorPlanningintests/test_blueprint.pycovers theSHOWround-trip with zero changes, a trigger-only change, a quota plus trigger change, and create plus transfer.tests/test_resource_types.pycovers the rejected trigger lists. The SQL and JSON fixtures carry triggers.tests/integration/data_provider/test_fetch_resource_simple.py --snowflake -k ResourceMonitorpasses against a live account.USE ROLE ACCOUNTADMIN,CREATE RESOURCE MONITOR E2E_RM CREDIT_QUOTA = 5 TRIGGERS ON 75 PERCENT DO NOTIFY ON 100 PERCENT DO SUSPEND,GRANT OWNERSHIP ... TO ROLE PROBE_RM_OWNER COPY CURRENT GRANTSUSE ROLE PROBE_RM_OWNER,ALTER RESOURCE MONITOR E2E_RM SET CREDIT_QUOTA = 7 TRIGGERS ON 50 PERCENT DO NOTIFY ON 90 PERCENT DO SUSPEND ON 110 PERCENT DO SUSPEND_IMMEDIATEsync_resourcesUSE ROLE PROBE_RM_OWNER,DROP RESOURCE MONITOR IF EXISTS E2E_RMKnown gap, not changed here
CREATE RESOURCE MONITOR ... FREQUENCY = MONTHLYwithoutSTART_TIMESTAMPfails with error 090259, "Must specify frequency and start time together." A monitor that declaresfrequencymust also declarestart_timestamp. This behavior predates this PR.