-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
Upgrade Kusto SDK to 2020-02-15 from 2019-05-15 #6838
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @jrauschenbusch
Thanks for this PR :)
Taking a look through this LGTM - I'll kick off the tests now 👍
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like there is a potential crash with the AsDatabase
call:
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x44a518b]
goroutine 3312 [running]:
github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/kusto.resourceArmKustoDatabaseCreateUpdate(0xc0008f5960, 0x4aa1ec0, 0xc0011fa500, 0x0, 0x0)
/Users/kt/hashi/tf/azure/azurerm/azurerm/internal/services/kusto/kusto_database_resource.go:97 +0xdab
@katbyte You're right. Shame on me 🤦 Did not tested it before... The domain model changed from API 2010-05-15 to API 2020-02-15, so i had to adjust it. I'll fix and test it and update this PR afterwards. |
4cb838e
to
00111fe
Compare
While fixing the related issues i've found a bug within the unmarshalling logic in the azure sdk: |
93c007b
to
42b8831
Compare
@katbyte I can proceed with this PR as soon as Azure/azure-sdk-for-go#9756 is merged. But before, a module update for the azure-sdk-for-go dependency must take place. |
@jrauschenbusch - happy to bump us up to 43 when its released :) |
Dependent on #7188 |
99bf011
to
37e1dbf
Compare
37e1dbf
to
c7925ad
Compare
@tombuildsstuff All Kusto resources are now updated and tested (ID parsers extracted, Unit tests available, resources adapted to 2020-02-15 API). As there are now 2 types of databases ( Error: Error creating or updating Kusto Database "ajo8fe-kusto-database" (Resource Group "ajo8fe-rg-2", Cluster "ajo8fecluster2"): kusto.DatabasesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="BadRequest" Message="Creating readonly following database directly is not supported, create attached database configuration instead."
on main.tf line 276, in resource "azurerm_kusto_database" "test":
276: resource "azurerm_kusto_database" "test" { The attached database configuration should be another TF resource. The only thing is that one could try to add principals to an existing follower database having a BUT:
|
@katbyte Test results are ok now. make acctests SERVICE='kusto' TESTTIMEOUT='120m'==> 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 ./azurerm/internal/services/kusto/tests/ -timeout 120m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN TestAccDataSourceAzureRMKustoCluster_basic
=== PAUSE TestAccDataSourceAzureRMKustoCluster_basic
=== RUN TestAccAzureRMKustoCluster_basic
=== PAUSE TestAccAzureRMKustoCluster_basic
=== RUN TestAccAzureRMKustoCluster_update
=== PAUSE TestAccAzureRMKustoCluster_update
=== RUN TestAccAzureRMKustoCluster_withTags
=== PAUSE TestAccAzureRMKustoCluster_withTags
=== RUN TestAccAzureRMKustoCluster_sku
=== PAUSE TestAccAzureRMKustoCluster_sku
=== RUN TestAccAzureRMKustoDatabasePrincipal_basic
=== PAUSE TestAccAzureRMKustoDatabasePrincipal_basic
=== RUN TestAccAzureRMKustoDatabase_basic
=== PAUSE TestAccAzureRMKustoDatabase_basic
=== RUN TestAccAzureRMKustoDatabase_softDeletePeriod
=== PAUSE TestAccAzureRMKustoDatabase_softDeletePeriod
=== RUN TestAccAzureRMKustoDatabase_hotCachePeriod
=== PAUSE TestAccAzureRMKustoDatabase_hotCachePeriod
=== RUN TestAccAzureRMKustoEventHubDataConnection_basic
=== PAUSE TestAccAzureRMKustoEventHubDataConnection_basic
=== CONT TestAccDataSourceAzureRMKustoCluster_basic
=== CONT TestAccAzureRMKustoDatabase_basic
=== CONT TestAccAzureRMKustoCluster_basic
=== CONT TestAccAzureRMKustoCluster_withTags
=== CONT TestAccAzureRMKustoDatabasePrincipal_basic
=== CONT TestAccAzureRMKustoEventHubDataConnection_basic
=== CONT TestAccAzureRMKustoCluster_sku
=== CONT TestAccAzureRMKustoCluster_update
=== CONT TestAccAzureRMKustoDatabase_hotCachePeriod
=== CONT TestAccAzureRMKustoDatabase_softDeletePeriod
--- PASS: TestAccDataSourceAzureRMKustoCluster_basic (1341.51s)
--- PASS: TestAccAzureRMKustoCluster_basic (1521.29s)
--- PASS: TestAccAzureRMKustoCluster_withTags (1532.44s)
--- PASS: TestAccAzureRMKustoDatabase_softDeletePeriod (1596.07s)
--- PASS: TestAccAzureRMKustoDatabase_hotCachePeriod (1596.12s)
--- PASS: TestAccAzureRMKustoDatabase_basic (1643.31s)
--- PASS: TestAccAzureRMKustoDatabasePrincipal_basic (1654.12s)
--- PASS: TestAccAzureRMKustoEventHubDataConnection_basic (1717.40s)
--- PASS: TestAccAzureRMKustoCluster_sku (2509.78s)
--- PASS: TestAccAzureRMKustoCluster_update (2857.73s)
PASS
ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/kusto/tests 2858.935s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jrauschenbusch - LGTM now! 👍
This has been released in version 2.15.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 2.15.0"
}
# ... other configuration ... |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
No description provided.