Skip to content

feat(resource_monitor): manage triggers, several notify users, and non-ACCOUNTADMIN owners - #86

Open
GClunies wants to merge 1 commit into
datacoves:mainfrom
GClunies:fix/resource-monitor-triggers-owner-notify-users
Open

GClunies wants to merge 1 commit into
datacoves:mainfrom
GClunies:fix/resource-monitor-triggers-owner-notify-users

Conversation

@GClunies

@GClunies GClunies commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

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.

Gap Before After
Triggers ResourceMonitor had no field for TRIGGERS. Notify and suspend thresholds could not be declared or read back. New triggers field: a list of {threshold, action} with action NOTIFY, SUSPEND, or SUSPEND_IMMEDIATE. The fetch reads it back from notify_at, suspend_at, and suspend_immediately_at.
Several notify users The fetch returned the SHOW string "A, B", so the plan rejected the monitor it read. The fetch splits the string into a list.
Owner The spec raised ResourceMonitors can only be created by ACCOUNTADMIN for any other owner. Create runs as ACCOUNTADMIN and transfers ownership to the declared owner. Update and drop run as the owning role.
resource_monitors:
  - name: WH_MONITOR
    owner: MONITOR_ADMIN
    credit_quota: 1000
    notify_users: [ALICE, BOB]
    triggers:
      - threshold: 75
        action: NOTIFY
      - threshold: 100
        action: SUSPEND
      - threshold: 110
        action: SUSPEND_IMMEDIATE

Why

We tested each Snowflake behavior against a live account before we wrote the code.

Statement or read Result What the code does
ALTER RESOURCE MONITOR X SET CREDIT_QUOTA = 7 TRIGGERS ON ... Accepted. TRIGGERS replaces every trigger. update_resource_monitor renders SET props, then the full TRIGGERS clause.
ALTER RESOURCE MONITOR X SET TRIGGERS ON ... Syntax error. TRIGGERS is not a SET property. update__default cannot render it, so the resource has its own update function.
UNSET TRIGGERS, a bare TRIGGERS, SET TRIGGERS = (), NOTRIGGERS All rejected. No statement removes every trigger. triggers: [] raises ValueError. Omit triggers to leave them unmanaged.
TRIGGERS ON 75.5 PERCENT DO NOTIFY Syntax error 001003. A non-integer threshold raises ValueError.
SHOW RESOURCE MONITORS notify_at A comma-separated list, for example '50%,75%'. One production account reports '100%,50%,75%,90%', which is not numeric order. Triggers are sorted by threshold and action on both sides of the diff.
SHOW RESOURCE MONITORS notify_users '' or 'A, B, C'. Split, strip, and sort.
ALTER or DROP on a monitor owned by another role, run as ACCOUNTADMIN Error 003001, insufficient privileges. Update and drop run as the owner.
GRANT OWNERSHIP ON RESOURCE MONITOR X TO ROLE R as ACCOUNTADMIN Accepted. Create runs as ACCOUNTADMIN with an ownership transfer when the owner differs.

Tested

  • Unit tests: TestResourceMonitorPlanning in tests/test_blueprint.py covers the SHOW round-trip with zero changes, a trigger-only change, a quota plus trigger change, and create plus transfer. tests/test_resource_types.py covers the rejected trigger lists. The SQL and JSON fixtures carry triggers.
  • Integration: tests/integration/data_provider/test_fetch_resource_simple.py --snowflake -k ResourceMonitor passes against a live account.
  • Live end to end, with a monitor owned by a custom role:
Step SQL Snowcap ran Re-plan
Create 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 GRANTS 0 changes
Change quota and triggers USE 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_IMMEDIATE 0 changes
Drop with sync_resources USE ROLE PROBE_RM_OWNER, DROP RESOURCE MONITOR IF EXISTS E2E_RM 0 changes

Known gap, not changed here

CREATE RESOURCE MONITOR ... FREQUENCY = MONTHLY without START_TIMESTAMP fails with error 090259, "Must specify frequency and start time together." A monitor that declares frequency must also declare start_timestamp. This behavior predates this PR.

…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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant