feat: Add option to create streams which reference EXTERNAL TABLEs #661
Conversation
…l-tables-to-streams
| resource.TestCheckResourceAttr("snowflake_stream.test_stream", "name", accName), | ||
| resource.TestCheckResourceAttr("snowflake_stream.test_stream", "database", accName), | ||
| resource.TestCheckResourceAttr("snowflake_stream.test_stream", "schema", accName), | ||
| resource.TestCheckResourceAttr("snowflake_stream.test_stream", "on_table", fmt.Sprintf("%s.%s.%s", accName, accName, "STREAM_ON_TABLE")), |
There was a problem hiding this comment.
Is this supposed to be on_external_table?
There was a problem hiding this comment.
And I think the name is supposed be accName, as well... not STREAM_ON_TABLE
There was a problem hiding this comment.
Hey @alldoami on_external_table is a bool, and creating a stream on an external table without ... EXTERNAL TABLE will raise an error; so, the acceptance test is simply ensuring that the table referenced by on_table (which is an external table created via externalTableStreamConfig) exists.
That said, I should definitely have added a check that on_external_table is true - see the most recent commits, which also resolve the STREAM_ON_TABLE reference - thanks! I'll have to set up an environment to run these tests locally which will reduce impact on you via these PRs, sorry.
|
/ok-to-test sha=eb335a8 |
|
Integration tests failure for eb335a8 |
|
@alldoami this should be resolved - I believe this was due to an extra param in the |
|
/ok-to-test sha=c58be0c |
|
Integration tests failure for c58be0c |
Co-authored-by: Allison Doami <allison.ox@gmail.com>
Co-authored-by: Allison Doami <allison.ox@gmail.com>
Co-authored-by: Allison Doami <allison.ox@gmail.com>
|
@alldoami Thank you! |
|
/ok-to-test sha=a356e27 |
|
Integration tests failure for a356e27 |
| } | ||
| ` | ||
|
|
||
| return fmt.Sprintf(s, name, name, name, name, locations, name) |
There was a problem hiding this comment.
If you don't want to reuse the config above, you'll have to create another random string to pass in so that the names are different. Just put
accNameExternalTable := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) below the definition ofaccName above
There was a problem hiding this comment.
And then replace the accNames with accNameExternalTable in your config above
There was a problem hiding this comment.
Ah I see - thank you again, see the most recent commits!
…at referenced name, db, schema are different between previous and new external tests
…omer/terraform-provider-snowflake into feature/add-external-tables-to-streams
|
/ok-to-test sha=c95a50f |
|
Integration tests failure for c95a50f |
| column { | ||
| name = "column2" | ||
| type = "TIMESTAMP_NTZ(9)" | ||
| as = "($1:'CreatedDate'::timestamp)" |
There was a problem hiding this comment.
Looks like it's not happy with this... Is this written correctly?
There was a problem hiding this comment.
Hmm, should be; I grabbed it directly from the external_table_acceptance_test.go test. I can dig in a bit tonight hopefully, sorry for the many issues with this test
|
How's this going? Just stumbled across this issue myself! |
|
@momer Please reopen when ready for another review. |
Allow the creation of
STREAMSonEXTERNAL TABLEs.Test Plan
external_tablewhich I modified a bit, so is a bit fragile.References