Skip to content
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

Altering Resource Monitors doesnt work as expected #1832

Open
curtisq opened this issue May 25, 2023 · 2 comments
Open

Altering Resource Monitors doesnt work as expected #1832

curtisq opened this issue May 25, 2023 · 2 comments
Assignees
Labels
bug Used to mark issues with provider's incorrect behavior category:resource resource:resource_monitor Issue connected to the snowflake_resource_monitor resource

Comments

@curtisq
Copy link

curtisq commented May 25, 2023

Provider Version

The provider version you are using.
0.55.1

Terraform Version

The version of Terraform you were using when the bug was encountered.

1.0.11

Describe the bug

Changing only the triggers of a resource monitor throws a syntax error

For example take the following monitor:

resource "snowflake_resource_monitor" "some_monitor" {
  name         = "some_monitor"
  warehouses   = [snowflake_warehouse.some_warehouse.name]
  credit_quota = 10

  frequency       = "WEEKLY"
  start_timestamp = "2023-02-02T00:00:00Z"

  notify_triggers            = [90]
  suspend_triggers           = [100]
  suspend_immediate_triggers = [105]

  notify_users = [snowflake_user.some_user.name]
}

If I were to say change a trigger line to notify_triggers = [95] and change nothing else then the provider produces the following SQL:

ALTER RESOURCE MONITOR "some_monitor" 
SET  
TRIGGERS ON 100 PERCENT DO SUSPEND 
    ON 105 PERCENT DO SUSPEND_IMMEDIATE 
    ON 95 PERCENT DO NOTIFY

This fails with

syntax error line 3 at position 24 unexpected 'DO'.
syntax error line 4 at position 4 unexpected 'ON'.
...

Expected behavior

A clear and concise description of what you expected to happen.

Based on the docs you must put something (not triggers or notify users) in the SET clause. The following SQL will work:

ALTER RESOURCE MONITOR "some_monitor" 
SET  CREDIT_QUOTA=10
TRIGGERS ON 100 PERCENT DO SUSPEND 
    ON 105 PERCENT DO SUSPEND_IMMEDIATE 
    ON 90 PERCENT DO NOTIFY

edit: The following will also work

ALTER RESOURCE MONITOR "some_monitor" 
TRIGGERS ON 100 PERCENT DO SUSPEND 
    ON 105 PERCENT DO SUSPEND_IMMEDIATE 
    ON 90 PERCENT DO NOTIFY

I would expect the provider to produce this SQL

Code samples and commands

Code samples are in the description already

Additional context

I assume the same issue exists for changing only the notify users but have not tested this.

@curtisq curtisq added the bug Used to mark issues with provider's incorrect behavior label May 25, 2023
@sfc-gh-jcieslak sfc-gh-jcieslak added category:resource resource:resource_monitor Issue connected to the snowflake_resource_monitor resource labels May 20, 2024
@sfc-gh-jcieslak
Copy link
Collaborator

Hey 👋
Please refer to this issue - #1500 (comment). I'll let you know when the updated resource is available.

@sfc-gh-jcieslak sfc-gh-jcieslak self-assigned this Sep 5, 2024
sfc-gh-jcieslak added a commit that referenced this issue Sep 11, 2024
## Changes
- Add ValuePresent assert to our custom assertions
- Add ToConfigValues function for every model
- Update Resource Monitor SDK + unit and integration tests
- Update Resource Monitor Resource + acc tests
- Handle issues connected to the resource monitor (mostly timestamp
format difference causing infinite plan or only trigger updates causing
SQL compilation error):
  -  #1500 
  - #1624 
  - #1716 
  - #1754 
  - #1821 
  - #1832 
  - #1990

## Next pr
- Adjust examples and update migration notes
- Data source (impl, tests, examples, migration notes)

## References
* [CREATE RESOURCE
MONITOR](https://docs.snowflake.com/en/sql-reference/sql/create-resource-monitor)
sfc-gh-fbudzynski pushed a commit that referenced this issue Sep 19, 2024
## Changes
- Add ValuePresent assert to our custom assertions
- Add ToConfigValues function for every model
- Update Resource Monitor SDK + unit and integration tests
- Update Resource Monitor Resource + acc tests
- Handle issues connected to the resource monitor (mostly timestamp
format difference causing infinite plan or only trigger updates causing
SQL compilation error):
  -  #1500 
  - #1624 
  - #1716 
  - #1754 
  - #1821 
  - #1832 
  - #1990

## Next pr
- Adjust examples and update migration notes
- Data source (impl, tests, examples, migration notes)

## References
* [CREATE RESOURCE
MONITOR](https://docs.snowflake.com/en/sql-reference/sql/create-resource-monitor)
@sfc-gh-jcieslak
Copy link
Collaborator

Hey 👋
The new and refactored resource monitor was released yesterday in version 0.96.0 of the provider. Please migrate with migration guide and let me know if the issue has been resolved so we could close the ticket. Thanks in advance 🙏.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to mark issues with provider's incorrect behavior category:resource resource:resource_monitor Issue connected to the snowflake_resource_monitor resource
Projects
None yet
Development

No branches or pull requests

2 participants