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.

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

New Resource azurerm_cosmosdb_sql_storedprocedure #6189

Merged

Conversation

jackbatzner
Copy link
Contributor

@jackbatzner jackbatzner commented Mar 19, 2020

Add new resource azurerm_cosmosdb_sql_storedprocedure to cover part of #5979

=== RUN TestAccAzureRMCosmosDbSqlStoredProcedure_basic
=== PAUSE TestAccAzureRMCosmosDbSqlStoredProcedure_basic
=== CONT TestAccAzureRMCosmosDbSqlStoredProcedure_basic
--- PASS: TestAccAzureRMCosmosDbSqlStoredProcedure_basic (1347.06s)

=== RUN TestAccAzureRMCosmosDbSqlStoredProcedure_update
=== PAUSE TestAccAzureRMCosmosDbSqlStoredProcedure_update
=== CONT TestAccAzureRMCosmosDbSqlStoredProcedure_update
--- PASS: TestAccAzureRMCosmosDbSqlStoredProcedure_update (1704.64s)

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @Brunhil

Thanks for this PR :)

Taking a look through this is looking pretty good - I've left some comments inline but if we can fix those up then this otherwise LGTM 👍

Thanks!

@@ -2,18 +2,24 @@ package client

import (
"github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2015-04-08/documentdb"
sqlResources "github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2019-08-01/documentdb"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason to only do this for the SQL Resources? in general we've found it best to use the same API version for all components of a service to ensure the service behaves correctly - as such it'd be good to update all of the resources if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason was to reduce potential for breaking changes on other resources. I'll work on cleaning up the other comments and get this one addressed last.

Great idea for keeping all resources in sync with the latest API!

Copy link
Contributor Author

@jackbatzner jackbatzner Mar 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been addressed in PR 6253. I will update the client in this PR when the other one gets merged in

go.mod Outdated Show resolved Hide resolved
website/docs/r/cosmosdb_sql_storedprocedure.html.markdown Outdated Show resolved Hide resolved
azurerm/internal/services/cosmos/registration.go Outdated Show resolved Hide resolved
website/docs/r/cosmosdb_sql_storedprocedure.html.markdown Outdated Show resolved Hide resolved
@jackbatzner jackbatzner changed the title New Resource azurerm_cosmosdb_sql_storedprocedure Upgrade Cosmos SDK to v2019-08-01 && New Resource azurerm_cosmosdb_sql_storedprocedure Mar 25, 2020
@jackbatzner jackbatzner force-pushed the azurerm_cosmosdb_sqlstoredprocedure branch from 2b4ad81 to 59b3339 Compare March 25, 2020 18:35
@jackbatzner jackbatzner changed the title Upgrade Cosmos SDK to v2019-08-01 && New Resource azurerm_cosmosdb_sql_storedprocedure New Resource azurerm_cosmosdb_sql_storedprocedure Mar 25, 2020
`, template, data.RandomInteger)
}

func testAccAzureRMCosmosDbSqlStoredProcedure_update(data acceptance.TestData) string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was having issues running when updating it locally. Will continue troubleshooting the issues locally before pushing again. Wanted to get the other changes reviewed first

@katbyte katbyte added the sdk/requires-newer-api-version This requires upgrading the version of the API being used label May 7, 2020
@katbyte
Copy link
Collaborator

katbyte commented May 7, 2020

blocked by #6253

@katbyte katbyte removed dependencies sdk/requires-newer-api-version This requires upgrading the version of the API being used labels Jul 24, 2020
@ghost ghost added size/XL and removed size/XXL labels Jul 24, 2020
@katbyte katbyte merged commit 93ac65d into hashicorp:master Sep 25, 2020
@katbyte katbyte added this to the v2.30.0 milestone Sep 25, 2020
katbyte added a commit that referenced this pull request Sep 25, 2020
@ghost
Copy link

ghost commented Oct 1, 2020

This has been released in version 2.30.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.30.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Oct 26, 2020

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!

@ghost ghost locked as resolved and limited conversation to collaborators Oct 26, 2020
@jackbatzner jackbatzner deleted the azurerm_cosmosdb_sqlstoredprocedure branch March 2, 2021 02:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants