-
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
new resource: azurerm_sentinel_data_connector_office_365
#10671
new resource: azurerm_sentinel_data_connector_office_365
#10671
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 @magodo
Thanks for this PR - I've taken a look through and left some (mostly minor) comments inline - if we can fix those up then this should be otherwise good to go 👍
Thanks!
azurerm/internal/services/sentinel/sentinel_data_connector_office_365.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/sentinel/sentinel_data_connector_office_365.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/sentinel/sentinel_data_connector_office_365.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/sentinel/sentinel_data_connector_office_365.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/sentinel/sentinel_data_connector_office_365.go
Outdated
Show resolved
Hide resolved
website/docs/r/sentinel_data_connector_office_365.html.markdown
Outdated
Show resolved
Hide resolved
website/docs/r/sentinel_data_connector_office_365.html.markdown
Outdated
Show resolved
Hide resolved
website/docs/r/sentinel_data_connector_office_365.html.markdown
Outdated
Show resolved
Hide resolved
website/docs/r/sentinel_data_connector_office_365.html.markdown
Outdated
Show resolved
Hide resolved
website/docs/r/sentinel_data_connector_office_365.html.markdown
Outdated
Show resolved
Hide resolved
azurerm/internal/services/sentinel/sentinel_data_connector_office_365.go
Show resolved
Hide resolved
83b7b3d
to
bdccbc9
Compare
Thank you for the comments! I've rebased the original commit onto your last merged base commit, and also have resolved the review comments but the etag one. Please take another review. |
Hey @magodo - looks l ike the tests are failing for all the new data connector PRs:
|
@katbyte I've fixed the error now. 💤 make testacc TEST=./azurerm/internal/services/sentinel TESTARGS='-run TestAccAzureRMSentinelDataConnectorOffice365_'
==> 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 ./azurerm/internal/services/sentinel -v -run TestAccAzureRMSentinelDataConnectorOffice365_ -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN TestAccAzureRMSentinelDataConnectorOffice365_basic
=== PAUSE TestAccAzureRMSentinelDataConnectorOffice365_basic
=== RUN TestAccAzureRMSentinelDataConnectorOffice365_complete
=== PAUSE TestAccAzureRMSentinelDataConnectorOffice365_complete
=== RUN TestAccAzureRMSentinelDataConnectorOffice365_update
=== PAUSE TestAccAzureRMSentinelDataConnectorOffice365_update
=== RUN TestAccAzureRMSentinelDataConnectorOffice365_requiresImport
=== PAUSE TestAccAzureRMSentinelDataConnectorOffice365_requiresImport
=== CONT TestAccAzureRMSentinelDataConnectorOffice365_basic
=== CONT TestAccAzureRMSentinelDataConnectorOffice365_requiresImport
=== CONT TestAccAzureRMSentinelDataConnectorOffice365_update
=== CONT TestAccAzureRMSentinelDataConnectorOffice365_complete
--- PASS: TestAccAzureRMSentinelDataConnectorOffice365_basic (180.22s)
--- PASS: TestAccAzureRMSentinelDataConnectorOffice365_requiresImport (186.31s)
--- PASS: TestAccAzureRMSentinelDataConnectorOffice365_complete (186.90s)
--- PASS: TestAccAzureRMSentinelDataConnectorOffice365_update (585.19s)
PASS
ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/sentinel 585.241s |
azurerm/internal/services/sentinel/sentinel_data_connector_office_365.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/sentinel/sentinel_data_connector_office_365.go
Outdated
Show resolved
Hide resolved
The etag related code can be removed once: Azure/azure-rest-api-specs#13203 is resolved. |
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 @magodo
Thanks for pushing those changes - taking a look through aside from a couple of minor comments this otherwise LGTM 👍
Thanks!
website/docs/r/sentinel_data_connector_office_365.html.markdown
Outdated
Show resolved
Hide resolved
website/docs/r/sentinel_data_connector_office_365.html.markdown
Outdated
Show resolved
Hide resolved
@tombuildsstuff I've updated accordingly, thank you! |
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.
LGTM 👍
This has been released in version 2.50.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.50.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 👉 hashibot-feedback@hashicorp.com. Thanks! |
new resource:
azurerm_sentinel_data_connector_office_365