fix: function not exist and integration grant #1154
Conversation
Fixed 'function' not exist scenario Fixed grant helpers to consider only relevant grants
Fixed 'function' not exist scenario Fixed grant helpers to consider only relevant grants Fixed 'stage' not exist scenario
israel
force-pushed
the
israel/fixing_more_resources
branch
from
3718433
to
a95879d
Compare
|
@israel i understand that you escalated this with Snowflake support. i would like to help you get this merged. if you could please address the two comments from my review i would be happy to merge with for you |
|
@israel if you are busy, would you like me to make the changes for you? |
@sfc-gh-swinkler, Sorry about the delay, was on vacation and just came back. |
pkg/resources/grant_helpers.go
Outdated
| log.Printf("[DEBUG] integration_name: %v", d.Get("integration_name")) | ||
| log.Printf("[DEBUG] privilege: %v", d.Get("privilege")) | ||
| log.Printf("[DEBUG] with_grant_option: %v", d.Get("with_grant_option")) | ||
| log.Printf("[DEBUG] enable_multiple_grants: %v", d.Get("enable_multiple_grants")) |
There was a problem hiding this comment.
these debug logs should be removed.
| } | ||
|
|
||
| // If no relevant grants, set id to blank and return (see HACK HACK comment above) | ||
| if len(relevantGrants) == 0 { |
There was a problem hiding this comment.
can you check error code rather than use a hack?
There was a problem hiding this comment.
I thought that returning empty ID is the hack. (from the comment above that mentions the HACK). Regardless, I think that the check is OK. There is no error code in this case. A grant (or grants were) was found but non relevant to our search since they do not match the privilege or grant option properties.
|
@israel can you see now? |
Fixed 'function' not exist scenario Fixed grant helpers to consider only relevant grants Fixed 'stage' not exist scenario removed unnecessary debug logs
|
/ok-to-test sha=412e14b |
|
Integration tests failure for 412e14b |
|
can you please rebase to master? tests are failing for a weird reason |
I have rebased before last push. Test is failing and I think that the test might be wrong, however, not sure. Why is it expected that the result is empty here: if we created a non empty stage resource here: ? |
|
#1220 i have a fix for this here. once this gets merged we can retry your integration tests |
|
/ok-to-test sha=ee833bb |
|
@israel thank you for your contribution. sorry it took so long to get this merged. |
|
Integration tests success for ee833bb |
|
Thanks !! |
|
@israel i need to revert the changes that were made the to |
|
https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/1229/files this was a draft PR someone else did to fix the problem, but not sure if needed or not. if these is needed i suppose we can make a follow up PR |
Added generic resource not found error
Fixed 'function' not exist scenario
Fixed grant helpers to consider only relevant grants