Add dynamic container arg to datafactory dataset #16350
Conversation
|
@katbyte can we prioritize approving + merging this please? it fixes a regression made in 3.1.0 thank you!!! |
|
So, I'm not sure where, but there should probably be some sort of testing that handles catching upstream structure or factory function changes in all downstream consumers. |
|
Also the only reason I have confidence in this being the fix, is that I built and consumed locally and it removed the error I was having. Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│ - hashicorp/azurerm in /Users/gkman/code/gocode/bin
│
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵ |
|
@tombuildsstuff @katbyte @jackofallops Just wanted to check in on this PR, I am surprised there are no standard reviewers added. So I figured tagging the highest few contributors made the most sense. |
There was a problem hiding this comment.
Thanks for the fix @GKMAN ! running the tests here i'm seeing a number of failures:
------- Stdout: -------
=== RUN TestAccDataFactoryDatasetBinary_blob_dynamics
=== PAUSE TestAccDataFactoryDatasetBinary_blob_dynamics
=== CONT TestAccDataFactoryDatasetBinary_blob_dynamics
testcase.go:110: Step 1/2 error: After applying this test step, the plan was not empty.
stdout:
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# azurerm_data_factory_dataset_binary.test will be updated in-place
~ resource "azurerm_data_factory_dataset_binary" "test" {
id = "/subscriptions/*******/resourceGroups/acctestRG-df-220421185101183709/providers/Microsoft.DataFactory/factories/acctestdf220421185101183709/datasets/acctestds220421185101183709"
name = "acctestds220421185101183709"
# (2 unchanged attributes hidden)
~ azure_blob_storage_location {
~ container = "@concat('foo/bar/',formatDateTime(convertTimeZone(utcnow(),'UTC','W. Europe Standard Time'),'yyyy-MM-dd'))" -> "@concat(azurerm_storage_container.test.name, '')"
# (5 unchanged attributes hidden)
}
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccDataFactoryDatasetBinary_blob_dynamics (210.32s)
FAIL
once fixed this should be good to merge
@katbyte oh dear, okay I'll take a look. Where/how are these tests run? I think I'm missing something. Everything was green build so I guess I just assumed everything was good. Can you elaborate on how the builds are green but the tests fail? Have I done something wrong? |
$ make acctests SERVICE=datafactory TESTARGS='-run=TestAccDataFactoryDataset' TESTTIMEOUT='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 -run=TestAccDataFactoryDataset -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN TestAccDataFactoryDatasetAzureBlob_basic
=== PAUSE TestAccDataFactoryDatasetAzureBlob_basic
=== RUN TestAccDataFactoryDatasetAzureBlob_dynamicPathAndFileName
=== PAUSE TestAccDataFactoryDatasetAzureBlob_dynamicPathAndFileName
=== RUN TestAccDataFactoryDatasetAzureBlob_update
=== PAUSE TestAccDataFactoryDatasetAzureBlob_update
=== RUN TestAccDataFactoryDatasetBinary_blob
=== PAUSE TestAccDataFactoryDatasetBinary_blob
=== RUN TestAccDataFactoryDatasetBinary_blob_dynamics
=== PAUSE TestAccDataFactoryDatasetBinary_blob_dynamics
=== RUN TestAccDataFactoryDatasetBinary_blob_with_filepath
=== PAUSE TestAccDataFactoryDatasetBinary_blob_with_filepath
=== RUN TestAccDataFactoryDatasetBinary_http
=== PAUSE TestAccDataFactoryDatasetBinary_http
=== RUN TestAccDataFactoryDatasetBinary_sftp
=== PAUSE TestAccDataFactoryDatasetBinary_sftp
=== RUN TestAccDataFactoryDatasetBinary_sftpComplete
=== PAUSE TestAccDataFactoryDatasetBinary_sftpComplete
=== RUN TestAccDataFactoryDatasetCosmosDbSQLAPI_basic
=== PAUSE TestAccDataFactoryDatasetCosmosDbSQLAPI_basic
=== RUN TestAccDataFactoryDatasetCosmosDbSQLAPI_update
=== PAUSE TestAccDataFactoryDatasetCosmosDbSQLAPI_update
=== RUN TestAccDataFactoryDatasetDelimitedText_http
=== PAUSE TestAccDataFactoryDatasetDelimitedText_http
=== RUN TestAccDataFactoryDatasetDelimitedText_http_update
=== PAUSE TestAccDataFactoryDatasetDelimitedText_http_update
=== RUN TestAccDataFactoryDatasetDelimitedText_blob_basic
=== PAUSE TestAccDataFactoryDatasetDelimitedText_blob_basic
=== RUN TestAccDataFactoryDatasetDelimitedText_blob
=== PAUSE TestAccDataFactoryDatasetDelimitedText_blob
=== RUN TestAccDataFactoryDatasetDelimitedText_blob_dynamics
=== PAUSE TestAccDataFactoryDatasetDelimitedText_blob_dynamics
=== RUN TestAccDataFactoryDatasetDelimitedText_blob_empty_path
=== PAUSE TestAccDataFactoryDatasetDelimitedText_blob_empty_path
=== RUN TestAccDataFactoryDatasetDelimitedText_blobFS
=== PAUSE TestAccDataFactoryDatasetDelimitedText_blobFS
=== RUN TestAccDataFactoryDatasetHTTP_basic
=== PAUSE TestAccDataFactoryDatasetHTTP_basic
=== RUN TestAccDataFactoryDatasetHTTP_update
=== PAUSE TestAccDataFactoryDatasetHTTP_update
=== RUN TestAccDataFactoryDatasetJSON_basic
=== PAUSE TestAccDataFactoryDatasetJSON_basic
=== RUN TestAccDataFactoryDatasetJSON_update
=== PAUSE TestAccDataFactoryDatasetJSON_update
=== RUN TestAccDataFactoryDatasetJSON_blob
=== PAUSE TestAccDataFactoryDatasetJSON_blob
=== RUN TestAccDataFactoryDatasetJSON_blob_dynamics
=== PAUSE TestAccDataFactoryDatasetJSON_blob_dynamics
=== RUN TestAccDataFactoryDatasetMySQL_basic
=== PAUSE TestAccDataFactoryDatasetMySQL_basic
=== RUN TestAccDataFactoryDatasetMySQL_update
=== PAUSE TestAccDataFactoryDatasetMySQL_update
=== RUN TestAccDataFactoryDatasetParquet_http
=== PAUSE TestAccDataFactoryDatasetParquet_http
=== RUN TestAccDataFactoryDatasetParquet_http_update
=== PAUSE TestAccDataFactoryDatasetParquet_http_update
=== RUN TestAccDataFactoryDatasetParquet_blob
=== PAUSE TestAccDataFactoryDatasetParquet_blob
=== RUN TestAccDataFactoryDatasetParquet_blob_dynamics
=== PAUSE TestAccDataFactoryDatasetParquet_blob_dynamics
=== RUN TestAccDataFactoryDatasetPostgreSQL_basic
=== PAUSE TestAccDataFactoryDatasetPostgreSQL_basic
=== RUN TestAccDataFactoryDatasetPostgreSQL_update
=== PAUSE TestAccDataFactoryDatasetPostgreSQL_update
=== RUN TestAccDataFactoryDatasetSnowflake_basic
=== PAUSE TestAccDataFactoryDatasetSnowflake_basic
=== RUN TestAccDataFactoryDatasetSnowflake_update
=== PAUSE TestAccDataFactoryDatasetSnowflake_update
=== RUN TestAccDataFactoryDatasetSQLServerTable_basic
=== PAUSE TestAccDataFactoryDatasetSQLServerTable_basic
=== RUN TestAccDataFactoryDatasetSQLServerTable_update
=== PAUSE TestAccDataFactoryDatasetSQLServerTable_update
=== CONT TestAccDataFactoryDatasetAzureBlob_basic
=== CONT TestAccDataFactoryDatasetHTTP_basic
=== CONT TestAccDataFactoryDatasetCosmosDbSQLAPI_basic
=== CONT TestAccDataFactoryDatasetParquet_http_update
=== CONT TestAccDataFactoryDatasetBinary_blob_dynamics
=== CONT TestAccDataFactoryDatasetBinary_blob_with_filepath
=== CONT TestAccDataFactoryDatasetBinary_sftp
=== CONT TestAccDataFactoryDatasetSnowflake_basic
=== CONT TestAccDataFactoryDatasetSQLServerTable_update
=== CONT TestAccDataFactoryDatasetPostgreSQL_update
=== CONT TestAccDataFactoryDatasetPostgreSQL_basic
=== CONT TestAccDataFactoryDatasetParquet_blob_dynamics
--- PASS: TestAccDataFactoryDatasetCosmosDbSQLAPI_basic (160.12s)
=== CONT TestAccDataFactoryDatasetParquet_blob
--- PASS: TestAccDataFactoryDatasetAzureBlob_basic (221.79s)
=== CONT TestAccDataFactoryDatasetAzureBlob_update
--- PASS: TestAccDataFactoryDatasetHTTP_basic (222.92s)
=== CONT TestAccDataFactoryDatasetBinary_blob
--- PASS: TestAccDataFactoryDatasetPostgreSQL_basic (224.38s)
=== CONT TestAccDataFactoryDatasetSQLServerTable_basic
--- PASS: TestAccDataFactoryDatasetSQLServerTable_update (226.79s)
=== CONT TestAccDataFactoryDatasetSnowflake_update
--- PASS: TestAccDataFactoryDatasetBinary_blob_dynamics (235.42s)
=== CONT TestAccDataFactoryDatasetJSON_blob_dynamics
--- PASS: TestAccDataFactoryDatasetParquet_blob_dynamics (239.34s)
=== CONT TestAccDataFactoryDatasetParquet_http
--- PASS: TestAccDataFactoryDatasetBinary_sftp (240.84s)
=== CONT TestAccDataFactoryDatasetMySQL_update
--- PASS: TestAccDataFactoryDatasetBinary_blob_with_filepath (255.63s)
=== CONT TestAccDataFactoryDatasetMySQL_basic
--- PASS: TestAccDataFactoryDatasetPostgreSQL_update (291.52s)
=== CONT TestAccDataFactoryDatasetDelimitedText_blob
--- PASS: TestAccDataFactoryDatasetParquet_http_update (327.62s)
=== CONT TestAccDataFactoryDatasetDelimitedText_blobFS
--- PASS: TestAccDataFactoryDatasetSQLServerTable_basic (143.31s)
=== CONT TestAccDataFactoryDatasetDelimitedText_blob_empty_path
--- PASS: TestAccDataFactoryDatasetParquet_blob (214.70s)
=== CONT TestAccDataFactoryDatasetDelimitedText_blob_dynamics
--- PASS: TestAccDataFactoryDatasetParquet_http (148.45s)
=== CONT TestAccDataFactoryDatasetDelimitedText_http_update
--- PASS: TestAccDataFactoryDatasetBinary_blob (165.42s)
=== CONT TestAccDataFactoryDatasetDelimitedText_blob_basic
=== CONT TestAccDataFactoryDatasetBinary_http
--- PASS: TestAccDataFactoryDatasetSnowflake_basic (395.24s)
--- PASS: TestAccDataFactoryDatasetJSON_blob_dynamics (166.14s)
=== CONT TestAccDataFactoryDatasetAzureBlob_dynamicPathAndFileName
--- PASS: TestAccDataFactoryDatasetMySQL_basic (190.91s)
=== CONT TestAccDataFactoryDatasetJSON_update
--- PASS: TestAccDataFactoryDatasetMySQL_update (223.55s)
=== CONT TestAccDataFactoryDatasetJSON_blob
--- PASS: TestAccDataFactoryDatasetAzureBlob_update (294.53s)
=== CONT TestAccDataFactoryDatasetJSON_basic
--- PASS: TestAccDataFactoryDatasetDelimitedText_blob (225.95s)
=== CONT TestAccDataFactoryDatasetDelimitedText_http
--- PASS: TestAccDataFactoryDatasetBinary_http (139.55s)
=== CONT TestAccDataFactoryDatasetHTTP_update
--- PASS: TestAccDataFactoryDatasetDelimitedText_blob_dynamics (166.10s)
=== CONT TestAccDataFactoryDatasetCosmosDbSQLAPI_update
--- PASS: TestAccDataFactoryDatasetDelimitedText_blobFS (247.32s)
=== CONT TestAccDataFactoryDatasetBinary_sftpComplete
--- PASS: TestAccDataFactoryDatasetDelimitedText_blob_empty_path (227.61s)
--- PASS: TestAccDataFactoryDatasetDelimitedText_blob_basic (208.73s)
--- PASS: TestAccDataFactoryDatasetAzureBlob_dynamicPathAndFileName (202.52s)
--- PASS: TestAccDataFactoryDatasetJSON_blob (158.33s)
--- PASS: TestAccDataFactoryDatasetDelimitedText_http (139.70s)
--- PASS: TestAccDataFactoryDatasetJSON_basic (142.08s)
--- PASS: TestAccDataFactoryDatasetSnowflake_update (439.56s)
--- PASS: TestAccDataFactoryDatasetJSON_update (240.84s)
--- PASS: TestAccDataFactoryDatasetDelimitedText_http_update (305.33s)
--- PASS: TestAccDataFactoryDatasetBinary_sftpComplete (139.61s)
--- PASS: TestAccDataFactoryDatasetHTTP_update (227.48s)
--- PASS: TestAccDataFactoryDatasetCosmosDbSQLAPI_update (294.72s)
PASS
ok github.com/hashicorp/terraform-provider-azurerm/internal/services/datafactory 837.158s |
|
@GKMAN, I just spent the morning doing exactly what you were doing and ended up merging my own code that does exactly what you were writing. After I merged, I went through the old PR and I saw your message about how you were already doing this work. I always want to prioritize contributors work but I ended up missing that here and I am so sorry for that. I hope there wasn't too much extra work on your end going into this PR and I'll definitely keep a better eye out for conflicting work going forward. |
|
@mbfrahry no worries, don't mind at all would rather have it fixed sooner than later, regardless of who fixes it. That explains why I all of a sudden had merge conflicts too. This got me to take the time to configure the I have just 1 question. Do the |
|
@mbfrahry That said I'll close this PR then, no harm no foul. |
|
@GKMAN when can we expect this to be released? |
|
@brandonros I don't know for sure, but it sure seems like releases are made basically weekly near the end of week. I would keep tabs on #16514 there's a bot that comments when a merged change has been released. |
|
Hey @GKMAN, thank you for your understanding! AccTests are not run through Github but our own CI so we only spin up infrastructure for things that we feel are ready to merge. The only tests being run through the Github checks are unit tests and linting checks which is why you wouldn't see the failures that @katbyte brought up! Hope that helps and again, I appreciate your understanding for missing this PR. |
|
@mbfrahry ya that makes total sense, and that explains the |
|
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. |
Fixes bug introduced by #15749