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/Data Source azurerm_data_share #6789

Merged
merged 13 commits into from May 21, 2020

Conversation

yupwei68
Copy link
Contributor

@yupwei68 yupwei68 commented May 6, 2020

fix #6480
datashare share new pass

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @yupwei68, overall this looks good with my main concern not exposing the snapshot's name in the block or to the user.

website/docs/r/data_share.html.markdown Outdated Show resolved Hide resolved
website/docs/r/data_share.html.markdown Outdated Show resolved Hide resolved
website/docs/r/data_share.html.markdown Outdated Show resolved Hide resolved
website/docs/r/data_share.html.markdown Outdated Show resolved Hide resolved
website/docs/d/data_share.html.markdown Outdated Show resolved Hide resolved
@katbyte katbyte added this to the v2.10.0 milestone May 11, 2020
yupwei68 added 2 commits May 12, 2020 10:47
@yupwei68 yupwei68 requested a review from katbyte May 14, 2020 07:17
@yupwei68
Copy link
Contributor Author

@katbyte , except read method still uses listcomplete for ImportStateVerify error, the createUpdate and delete method have changed. Please continue reviewing.

=== RUN   TestAccAzureRMDataShare_basic
=== PAUSE TestAccAzureRMDataShare_basic
=== CONT  TestAccAzureRMDataShare_basic
--- PASS: TestAccAzureRMDataShare_basic (352.64s)
=== RUN   TestAccAzureRMDataShare_requiresImport
=== PAUSE TestAccAzureRMDataShare_requiresImport
=== CONT  TestAccAzureRMDataShare_requiresImport
--- PASS: TestAccAzureRMDataShare_requiresImport (356.65s)
=== RUN   TestAccAzureRMDataShare_complete
=== PAUSE TestAccAzureRMDataShare_complete
=== CONT  TestAccAzureRMDataShare_complete
--- PASS: TestAccAzureRMDataShare_complete (351.99s)
=== RUN   TestAccAzureRMDataShare_update
=== PAUSE TestAccAzureRMDataShare_update
=== CONT  TestAccAzureRMDataShare_update
--- PASS: TestAccAzureRMDataShare_update (418.62s)
=== RUN   TestAccAzureRMDataShare_snapshotSchedule
=== PAUSE TestAccAzureRMDataShare_snapshotSchedule
=== CONT  TestAccAzureRMDataShare_snapshotSchedule
--- PASS: TestAccAzureRMDataShare_snapshotSchedule (604.38s)
PASS

@ghost ghost removed the waiting-response label May 14, 2020
@katbyte katbyte modified the milestones: v2.10.0, v2.11.0 May 14, 2020
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

thanks for the changes @yupwei68. Aside for a few comments i've left inline this is looking good.

It also looks like there is a failing test:

 TestAccDataSourceAzureRMDataShare_snapshotSchedule: testing.go:640: Step 1 error: 3 problems:
        
        - Provider produced invalid object: Provider "azurerm" planned an invalid value for data.azurerm_data_share.test during refresh: .timeouts: unsupported attribute "update".
        
        This is a bug in the provider, which should be reported in the provider's own issue tracker.
        - Provider produced invalid object: Provider "azurerm" planned an invalid value for data.azurerm_data_share.test during refresh: .timeouts: unsupported attribute "create".
        
        This is a bug in the provider, which should be reported in the provider's own issue tracker.
        - Provider produced invalid object: Provider "azurerm" planned an invalid value for data.azurerm_data_share.test during refresh: .timeouts: unsupported attribute "delete".
        

website/docs/d/data_share.html.markdown Outdated Show resolved Hide resolved
website/docs/d/data_share.html.markdown Outdated Show resolved Hide resolved
website/docs/d/data_share.html.markdown Outdated Show resolved Hide resolved
@yupwei68
Copy link
Contributor Author

Hi @katbyte ,thanks for your comments. Except one concern, corresponding changes have been pushed. Please continue reviewing.

resource_test

=== RUN   TestAccAzureRMDataShare_basic
=== PAUSE TestAccAzureRMDataShare_basic
=== CONT  TestAccAzureRMDataShare_basic
--- PASS: TestAccAzureRMDataShare_basic (352.89s)
=== RUN   TestAccAzureRMDataShare_requiresImport
=== PAUSE TestAccAzureRMDataShare_requiresImport
=== CONT  TestAccAzureRMDataShare_requiresImport
--- PASS: TestAccAzureRMDataShare_requiresImport (360.01s)
=== RUN   TestAccAzureRMDataShare_complete
=== PAUSE TestAccAzureRMDataShare_complete
=== CONT  TestAccAzureRMDataShare_complete
--- PASS: TestAccAzureRMDataShare_complete (352.88s)
=== RUN   TestAccAzureRMDataShare_update
=== PAUSE TestAccAzureRMDataShare_update
=== CONT  TestAccAzureRMDataShare_update
--- PASS: TestAccAzureRMDataShare_update (434.55s)
=== RUN   TestAccAzureRMDataShare_snapshotSchedule
=== PAUSE TestAccAzureRMDataShare_snapshotSchedule
=== CONT  TestAccAzureRMDataShare_snapshotSchedule
--- PASS: TestAccAzureRMDataShare_snapshotSchedule (604.04s)

data_source_test

=== RUN   TestAccDataSourceAzureRMDataShare_basic
=== PAUSE TestAccDataSourceAzureRMDataShare_basic
=== CONT  TestAccDataSourceAzureRMDataShare_basic
--- PASS: TestAccDataSourceAzureRMDataShare_basic (359.62s)
=== RUN   TestAccDataSourceAzureRMDataShare_snapshotSchedule
=== PAUSE TestAccDataSourceAzureRMDataShare_snapshotSchedule
=== CONT  TestAccDataSourceAzureRMDataShare_snapshotSchedule
--- PASS: TestAccDataSourceAzureRMDataShare_snapshotSchedule (432.83s)

@ghost ghost removed the waiting-response label May 20, 2020
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

thanks @yupwei68! LGTM 👍

@katbyte katbyte merged commit 6595a4c into hashicorp:master May 21, 2020
katbyte added a commit that referenced this pull request May 21, 2020
@ghost
Copy link

ghost commented May 22, 2020

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

@ghost
Copy link

ghost commented Jun 21, 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 👉 hashibot-feedback@hashicorp.com. Thanks!

@hashicorp hashicorp locked and limited conversation to collaborators Jun 21, 2020
@yupwei68 yupwei68 deleted the wyp-datashare-share branch July 10, 2020 02:04
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.

Support for [Data Share]
3 participants