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.

Sign up for GitHub

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

Jump to bottom

fix: Fix some bugs #2421

Merged
merged 5 commits into from
Jan 26, 2024
Merged

fix: Fix some bugs #2421

merged 5 commits into from
Jan 26, 2024

Conversation

sfc-gh-asawicki
Copy link
Collaborator

@sfc-gh-asawicki sfc-gh-asawicki commented Jan 25, 2024 •
edited
Loading

Fix a few small bugs

All of them have appropriate tests showing the error before and correct behavior after.

Fix blocked roles issue in oauth integration (#2358)

BLOCKED_ROLES_LIST was cast to wrong type which caused panic (https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/pkg/resources/oauth_integration.go#L313).

Fix database replication setup in resource (#2369)

No old config for blocked roles was causing empty list access (

terraform-provider-snowflake/pkg/resources/database.go

Line 271 in e3d086e

oldAccounts := oldConfig.([]interface{})[0].(map[string]interface{})["accounts"].([]interface{})
). Some other errors were discovered, so implementation was slightly improved.

Do not read query for dynamic table (#2329)

Cutting query from returned DDL was too greedy (

terraform-provider-snowflake/pkg/resources/dynamic_table.go

Line 204 in e3d086e

query := strings.TrimSpace(text[strings.Index(text, "AS")+3:])
). It is not that simple to read the right part. We may skip reading it, though, but it will make the import not fully functional. At the end, the matching was improved to handle more cases and appropriate test cases were added. The full explanation in https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/2421/files#diff-33718834696cbceb48b07f4a6aa4f739d1aa9052e190996a309f650c2e5a9edcR258.

Fix incorrectly merged streamlits

Formatting and createStage usage.

Fix #2358
Fix #2369
Fix #2329

@sfc-gh-asawicki sfc-gh-asawicki mentioned this pull request Jan 25, 2024
Closed
@github-actions GitHub Actions
Copy link

Integration tests failure for b53ca8e666f6d610fb881ad912b0b99894f5357c

@sfc-gh-asawicki sfc-gh-asawicki force-pushed the fix-some-bugs-2369-2358-2329 branch from b53ca8e to ea61380 Compare January 25, 2024 14:29
@sfc-gh-asawicki sfc-gh-asawicki marked this pull request as ready for review January 25, 2024 14:29
@github-actions GitHub Actions
Copy link

Integration tests failure for ea61380b9a5a3b3dc8197ddae3223a9f589d5a60

2 similar comments
@github-actions GitHub Actions
Copy link

Integration tests failure for ea61380b9a5a3b3dc8197ddae3223a9f589d5a60

@github-actions GitHub Actions
Copy link

Integration tests failure for ea61380b9a5a3b3dc8197ddae3223a9f589d5a60

@sfc-gh-asawicki sfc-gh-asawicki marked this pull request as draft January 25, 2024 17:14
@github-actions GitHub Actions
Copy link

Integration tests failure for 234130d944d7c32b6968acb4d34aae7eee2c1ca2

@sfc-gh-asawicki sfc-gh-asawicki force-pushed the fix-some-bugs-2369-2358-2329 branch from 234130d to 9790594 Compare January 26, 2024 10:18
@github-actions GitHub Actions
Copy link

Integration tests failure for 979059493053cae5ccb4eef5754517e1a25fca6c

@sfc-gh-asawicki sfc-gh-asawicki marked this pull request as ready for review January 26, 2024 10:59
@github-actions GitHub Actions
Copy link

Integration tests failure for 77e2454049bd92fc08de015b65f1bcf7fd581de0

@sfc-gh-asawicki sfc-gh-asawicki merged commit dec7cd9 into main Jan 26, 2024
8 of 9 checks passed
@sfc-gh-asawicki sfc-gh-asawicki deleted the fix-some-bugs-2369-2358-2329 branch January 26, 2024 11:46
@snowflake-release-please snowflake-release-please bot mentioned this pull request Jan 25, 2024
Merged
@chrisluedtke chrisluedtke mentioned this pull request Jan 26, 2024
Closed
@sonya sonya mentioned this pull request Jan 27, 2024
Merged
2 tasks
sfc-gh-jcieslak pushed a commit that referenced this pull request Feb 2, 2024
@snowflake-release-please
chore(main): release 0.85.0 (#2401)
ca8ca63 
🤖 I have created a release *beep* *boop*
---


##
[0.85.0](v0.84.1...v0.85.0)
(2024-02-01)


### 🎉 **What's new:**

* Add API integration to the SDK
([#2409](#2409))
([23acda5](23acda5))
* add application to sdk
([#2350](#2350))
([de97ad8](de97ad8))
* add external funcs to sdk
([#2440](#2440))
([c8cf09b](c8cf09b))
* Add grant privileges to share resource
([#2447](#2447))
([d8241a5](d8241a5))
* Add materialized view to the SDK
([#2403](#2403))
([a5ce699](a5ce699))
* Add notification integration to the SDK
([#2412](#2412))
([d84240c](d84240c))
* add sequences to sdk
([#2351](#2351))
([d2e5ffd](d2e5ffd))
* add snowflake grant privileges to account role
([#2365](#2365))
([e3d086e](e3d086e))
* add streamlits to sdk
([#2400](#2400))
([129d24c](129d24c))
* add-call-with to sdk
([#2337](#2337))
([ebcd1bc](ebcd1bc))
* stages migration follow-up
([#2372](#2372))
([3939dbe](3939dbe))
* Use API integration from SDK
([#2429](#2429))
([1ccc864](1ccc864))
* Use managed account from the SDK
([#2420](#2420))
([3aaa080](3aaa080))
* Use materialized views and views from SDK
([#2448](#2448))
([dc66d02](dc66d02))
* Use notification integration from sdk
([#2445](#2445))
([e8915cc](e8915cc))
* use roles from the SDK
([#2405](#2405))
([c645b4d](c645b4d))
* Use row access policy from SDK
([#2428](#2428))
([119af5e](119af5e))
* Use SDK in the storage integration
([#2380](#2380))
([ce0741c](ce0741c))
* use sequence from sdk and add ordering attr
([#2419](#2419))
([973b8f7](973b8f7)),
closes
[#2387](#2387)
* Use stage from sdk
([#2427](#2427))
([c17effd](c17effd))


### 🔧 **Misc**

* add missing deprecation message
([#2451](#2451))
([77de569](77de569))


### 🐛 **Bug fixes:**

* account role test
([#2422](#2422))
([c1b47d1](c1b47d1))
* Adjust tests after Snowflake behavior change
([#2404](#2404))
([8c03ffb](8c03ffb))
* app-pkg unset
([#2399](#2399))
([fedb1df](fedb1df))
* Fix some bugs
([#2421](#2421))
([dec7cd9](dec7cd9)),
closes
[#2358](#2358)
[#2369](#2369)
[#2329](#2329)
* snowflake_grant_privileges_to_role read
([#2424](#2424))
([5385cec](5385cec))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
sfc-gh-asawicki added a commit that referenced this pull request Feb 12, 2024
@sonya @sfc-gh-asawicki
fix: parse dynamic table query from DDL (#2438)
d76815c 
Previously we have been extracting the `query` portion of dynamic table
definitions from the stored DDL in Snowflake by searching for the string
position of keywords that preceded the query. However, this approach has
proven inadequate as there are many ways to write working dynamic table
definitions that conflict with string position assumptions.
    
Observing that definitions of views and materialized views have faced
similar challenges but are currently working, this commit extends the
existing ViewSelectStatementExtractor that is currently being used to
extract the query definition from view and materialized view DDL
statements, applying the same approach to dynamic table DDL statements.

This is an attempt to address #2329

## Test Plan
* [x] acceptance tests
* [x] testing in our existing stack. We have an existing stack with ~100
dynamic tables where many have CTEs and fail to be parsed with v0.85.0.
A few contains `AS` in the name and were affected by spurious diffs with
v0.78.0 (as observed in #2329). All of our dynamic tables are properly
preserved with code change in this PR.

## References

Original issue:
#2329
Previous attempt to fix:
#2421

---------

Co-authored-by: Artur Sawicki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-jcieslak sfc-gh-jcieslak sfc-gh-jcieslak approved these changes

@sfc-gh-swinkler sfc-gh-swinkler Awaiting requested review from sfc-gh-swinkler

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@sfc-gh-asawicki @sfc-gh-jcieslak