Fix schema_column for azurerm_data_factory_dataset_snowflake resource #13344

orestiskats19 · 2021-09-14T14:00:30Z

Acceptance Tests:

make acctests SERVICE='datafactory/data_factory_dataset_snowflake_resource_test.go' TEST
TIMEOUT='60m'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./internal/services/datafactory/data_factory_dataset_snowflake_resource_test.go  -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccDataFactoryDatasetSnowflake_basic
=== PAUSE TestAccDataFactoryDatasetSnowflake_basic
=== RUN   TestAccDataFactoryDatasetSnowflake_update
=== PAUSE TestAccDataFactoryDatasetSnowflake_update
=== CONT  TestAccDataFactoryDatasetSnowflake_basic
=== CONT  TestAccDataFactoryDatasetSnowflake_update
--- PASS: TestAccDataFactoryDatasetSnowflake_basic (265.45s)
--- PASS: TestAccDataFactoryDatasetSnowflake_update (319.66s)
PASS
ok      command-line-arguments  320.973s

Merge main master from fork

Merge with master

Merge new releases

katbyte

Thanks for this fix @orestiskats19 - have a couple quesions/comments i've left inline

katbyte · 2021-09-14T14:12:55Z

internal/services/datafactory/data_factory_dataset_snowflake_resource.go

-								"Byte",
-								"Byte[]",
-								"Boolean",
-								"Date",
-								"DateTime",
-								"DateTimeOffset",
-								"Decimal",
-								"Double",
-								"Guid",
-								"Int16",
-								"Int32",
-								"Int64",
-								"Single",
-								"String",
-								"TimeSpan",


we cannot remove these until 3.0 - can we add a comment to that affect and leave these in?

Thanks for reviewing my PR @katbyte. Why we can't replace those with the right data types of Snowflake? Don't think it works as it is now...

someone might have a confid with it somewhere, that maybe hasn't run for a while and used to work and the API is returning these old values - we don't want tf to suddenly fail so we need to keep them around until 3.0 (which is in the next 6 months) - as such this would be a breaking change we can only make then

katbyte · 2021-09-14T14:13:08Z

internal/services/datafactory/data_factory_dataset_snowflake_resource.go

 							}, false),
 						},
-						"description": {


how come we are removing the description property?

Seems that the description property is used only for the structure column (used in other resources) and not for the snowflake schema column. For every column in Snowflake the properties are name, type, precision and scale.

ahh ok. as above removing this is a breaking change even if it currently does nothing, could we instead deprecate it and we'll remove it in 3.0 - thanks!

…umn to structure_column

orestiskats19 · 2021-09-21T11:52:37Z

Thanks @katbyte. I did a step back and made it backwards compatible with structure column. It is renamed now from schema_column to structure_column.

Acceptance tests:

==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./internal/services/datafactory/data_factory_dataset_snowflake_resource_test.go  -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccDataFactoryDatasetSnowflake_basic
=== PAUSE TestAccDataFactoryDatasetSnowflake_basic
=== RUN   TestAccDataFactoryDatasetSnowflake_update
=== PAUSE TestAccDataFactoryDatasetSnowflake_update
=== CONT  TestAccDataFactoryDatasetSnowflake_basic
=== CONT  TestAccDataFactoryDatasetSnowflake_update
--- PASS: TestAccDataFactoryDatasetSnowflake_basic (221.14s)
--- PASS: TestAccDataFactoryDatasetSnowflake_update (303.24s)
PASS
ok      command-line-arguments  304.599s

katbyte

Thanks @orestiskats19 - overall looks great! however i'm guessing the intent is to replace the old colum with the new one so we should probably deprecate it and point users to the new one?

katbyte · 2021-09-23T02:00:29Z

internal/services/datafactory/data_factory_dataset_snowflake_resource.go

@@ -107,7 +107,7 @@ func resourceDataFactoryDatasetSnowflake() *pluginsdk.Resource {
 				},
 			},

-			"schema_column": {
+			"structure_column": {


we should probably deprecate this property?

Thanks @katbyte, I added the deprecated warning.

katbyte

Thanks for making those changes @orestiskats19 - this LGTM now 🚀

…resource (#13344)" This reverts commit b1b3374.

katbyte · 2021-09-23T22:44:28Z

For some additional context - after merge when preparing the changelog message i realized this is a breaking change as the wrong block was being assigned to the wrong property. After toying with changing it/reverting it back to its mostly broken state we decided to keep this as is and detail it in the upgrade notes. If you wish to retain the old behaviour please change schema_column to structure_column.

github-actions · 2021-09-28T09:02:35Z

This functionality has been released in v2.78.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2021-10-29T02:05:49Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Orestis Katsanakis added 13 commits April 18, 2021 21:23

Merge pull request #1 from terraform-providers/master

14f09d6

Merge main master from fork

Merge pull request #2 from terraform-providers/master

440b596

Merge with master

Merge pull request #4 from terraform-providers/master

3eeefb1

Merge new releases

Merge branch 'terraform-providers:main' into master

1408299

Merge branch 'terraform-providers:main' into master

7fdc8eb

replace schema properties, add docs and fix tests

c6c44b3

formatting

c23314e

fix undefined schema issue

d22e46d

add schema expand

60685a8

update data types and docs

e6aaca7

fix fmt

41404ca

fix tests update1 & 2

5c4f1de

fix check value for schema columns tests

0b5dcc2

github-actions bot added documentation size/L labels Sep 14, 2021

katbyte requested changes Sep 14, 2021

View reviewed changes

katbyte added bug enhancement service/data-factory labels Sep 17, 2021

katbyte added this to the v2.78.0 milestone Sep 17, 2021

katbyte added the waiting-response label Sep 17, 2021

make it backwards compatible with structure column, rename schema_col…

75f59ea

…umn to structure_column

github-actions bot removed the waiting-response label Sep 21, 2021

katbyte requested changes Sep 23, 2021

View reviewed changes

add deprecated warning for structure column

4c485b2

katbyte approved these changes Sep 23, 2021

View reviewed changes

katbyte merged commit b1b3374 into hashicorp:main Sep 23, 2021

katbyte added a commit that referenced this pull request Sep 23, 2021

Revert "Fix schema_column for azurerm_data_factory_dataset_snowflake …

1f66268

…resource (#13344)" This reverts commit b1b3374.

katbyte mentioned this pull request Sep 23, 2021

Revert "Fix schema_column for azurerm_data_factory_dataset_snowflake resource" #13485

Closed

katbyte added a commit that referenced this pull request Sep 23, 2021

CHANGELOG.md for #13344

386977b

github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix schema_column for azurerm_data_factory_dataset_snowflake resource #13344

Fix schema_column for azurerm_data_factory_dataset_snowflake resource #13344

orestiskats19 commented Sep 14, 2021 •

edited

Loading

katbyte left a comment

katbyte Sep 14, 2021

orestiskats19 Sep 14, 2021

katbyte Sep 17, 2021 •

edited

Loading

katbyte Sep 14, 2021

orestiskats19 Sep 14, 2021

katbyte Sep 17, 2021

orestiskats19 commented Sep 21, 2021

katbyte left a comment

katbyte Sep 23, 2021

orestiskats19 Sep 23, 2021

katbyte left a comment

katbyte commented Sep 23, 2021

github-actions bot commented Sep 28, 2021

github-actions bot commented Oct 29, 2021

Fix schema_column for azurerm_data_factory_dataset_snowflake resource #13344

Fix schema_column for azurerm_data_factory_dataset_snowflake resource #13344

Conversation

orestiskats19 commented Sep 14, 2021 • edited Loading

katbyte left a comment

Choose a reason for hiding this comment

katbyte Sep 14, 2021

Choose a reason for hiding this comment

orestiskats19 Sep 14, 2021

Choose a reason for hiding this comment

katbyte Sep 17, 2021 • edited Loading

Choose a reason for hiding this comment

katbyte Sep 14, 2021

Choose a reason for hiding this comment

orestiskats19 Sep 14, 2021

Choose a reason for hiding this comment

katbyte Sep 17, 2021

Choose a reason for hiding this comment

orestiskats19 commented Sep 21, 2021

katbyte left a comment

Choose a reason for hiding this comment

katbyte Sep 23, 2021

Choose a reason for hiding this comment

orestiskats19 Sep 23, 2021

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

katbyte commented Sep 23, 2021

github-actions bot commented Sep 28, 2021

github-actions bot commented Oct 29, 2021

orestiskats19 commented Sep 14, 2021 •

edited

Loading

katbyte Sep 17, 2021 •

edited

Loading