-
Notifications
You must be signed in to change notification settings - Fork 427
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
fix: refactor ReadWarehouse function to correctly read object parameters #745
Conversation
Could you give a bit more description on what you did in this PR? |
Hello @alldoami . Updated PR description to give more details on this issue. I hope that makes it clearer. We have encountered this bug while trying to upgrade |
/ok-to-test sha=8e1ad2b |
Integration tests failure for 8e1ad2b |
Hello @alldoami. I would like to fix integration tests but don't know how to test and work on it locally. Requires account and username to run tests. Could you please help with this? |
You can set these account variables: https://github.com/chanzuckerberg/terraform-provider-snowflake#pull-request-ci |
Hello @alldoami . I updated integration tests and run |
@@ -105,7 +106,7 @@ var warehouseSchema = map[string]*schema.Schema{ | |||
"statement_timeout_in_seconds": { | |||
Type: schema.TypeInt, | |||
Optional: true, | |||
Default: 0, | |||
Default: 172800, |
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.
Default value for this parameter is 172800
(https://docs.snowflake.com/en/sql-reference/parameters.html#statement-timeout-in-seconds). I think provider defaults should be the same as Snowflake defaults. What do you think?
Should fix #601 |
/ok-to-test sha=d9e3c20 |
Integration tests success for d9e3c20 |
Fixes #739
#598 has introduced 3 additional parameters (
statement_timeout_in_seconds
,statement_queued_timeout_in_seconds
andmax_concurrency_level
) for warehouse resource. After adding this feature,terraform plan
has started to consistently generate diff for these parameters even when they are already applied. Currently, provider is usingSHOW WAREHOUSES LIKE $warehouse_name
query but according to doc, these columns are not included in the query output. That is why generated diff always reportszero-value
for these fields.After this update, ReadWarehouse function will run the additional query (
SHOW PARAMETERS IN WAREHOUSE $warehouse_name
) to fetch warehouse parameters. Not sure if this is the best way but I created a new struct (WarehouseBuilder
) by embedding/overriding generic Builder (TaskBuilder uses a similar approach). Updatingschema.ResourceData
by using the output of this query does not generate diff anymore.