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_machine_learning_datastore_blobstorage #19909

Merged
merged 19 commits into from
Jan 20, 2023

Conversation

xuzhang3
Copy link
Contributor

@xuzhang3 xuzhang3 commented Jan 9, 2023

New resource for Machine learning workspace data store - blob storage

=== PAUSE TestAccMachineLearningDataStoreBlobStorage_accountKey
=== RUN   TestAccMachineLearningDataStoreBlobStorage_sasToken
=== PAUSE TestAccMachineLearningDataStoreBlobStorage_sasToken
=== RUN   TestAccMachineLearningDataStoreBlobStorage_Update
=== PAUSE TestAccMachineLearningDataStoreBlobStorage_Update
=== CONT  TestAccMachineLearningDataStoreBlobStorage_accountKey
=== CONT  TestAccMachineLearningDataStoreBlobStorage_Update
=== CONT  TestAccMachineLearningDataStoreBlobStorage_sasToken
--- PASS: TestAccMachineLearningDataStoreBlobStorage_accountKey (603.63s)
--- PASS: TestAccMachineLearningDataStoreBlobStorage_sasToken (633.35s)
--- PASS: TestAccMachineLearningDataStoreBlobStorage_Update (791.39s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/machinelearning       832.664s

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.

have a test failure, the delete might need to wait

------- Stdout: -------
=== RUN   TestAccMachineLearningDataStoreBlobStorage_Update
=== PAUSE TestAccMachineLearningDataStoreBlobStorage_Update
=== CONT  TestAccMachineLearningDataStoreBlobStorage_Update
    testing_new.go:84: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: deleting Resource Group "acctestRG-ml-230109183301437055": the Resource Group still contains Resources.
        
        Terraform is configured to check for Resources within the Resource Group when deleting the Resource Group - and
        raise an error if nested Resources still exist to avoid unintentionally deleting these Resources.
        
        Terraform has detected that the following Resources still exist within the Resource Group:
        
        * `/subscriptions/*******/resourceGroups/acctestRG-ml-230109183301437055/providers/microsoft.alertsmanagement/smartDetectorAlertRules/Failure Anomalies - acctestai-230109183301437055`
        * `/subscriptions/*******/resourceGroups/acctestRG-ml-230109183301437055/providers/microsoft.insights/actiongroups/Application Insights Smart Detection`
        
        This feature is intended to avoid the unintentional destruction of nested Resources provisioned through some
        other means (for example, an ARM Template Deployment) - as such you must either remove these Resources, or
        disable this behaviour using the feature flag `prevent_deletion_if_contains_resources` within the `features`
        block when configuring the Provider, for example:
        
        provider "azurerm" {
          features {
            resource_group {
              prevent_deletion_if_contains_resources = false
            }
          }
        }
        
        When that feature flag is set, Terraform will skip checking for any Resources within the Resource Group and
        delete this using the Azure API directly (which will clear up any nested resources).
        
        More information on the `features` block can be found in the documentation:
        https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs#features
        
        
--- FAIL: TestAccMachineLearningDataStoreBlobStorage_Update (1284.92s)
FAIL

@xuzhang3
Copy link
Contributor Author

@katbyte Machine learning workspace blob store APIs only have sync APIs. Test failure is because the application insight triggering the smart detection alert which is not covered by this PR. Do we need to fix this?

Copy link
Member

@stephybun stephybun 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 this @xuzhang3. I left some suggestions in-line, also this resource still needs to be converted to a typed resource as requested in the review comment left on the initial PR

@xuzhang3 xuzhang3 requested a review from stephybun January 18, 2023 12:35
@xuzhang3
Copy link
Contributor Author

@stephybun Thanks for reviewing this PR. Refactored into typed resource.

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.

LGTM 🧶

@katbyte katbyte merged commit 2f406a5 into hashicorp:main Jan 20, 2023
katbyte added a commit that referenced this pull request Jan 20, 2023
@github-actions github-actions bot added this to the v3.40.0 milestone Jan 20, 2023
@github-actions
Copy link

This functionality has been released in v3.40.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
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 20, 2023
@xuzhang3 xuzhang3 deleted the f/datastore_azureblob branch August 14, 2024 02:44
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