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.

Sign up for GitHub

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

Jump to bottom

azurerm_signalr_service - add http_request_logs_enabled property #21032

Merged
merged 6 commits into from
Apr 1, 2023
Merged

azurerm_signalr_service - add http_request_logs_enabled property #21032

merged 6 commits into from
Apr 1, 2023

Conversation

xiaxyi
Copy link
Contributor

@xiaxyi xiaxyi commented Mar 20, 2023

Adding support for property resource_logs that was introduced in 2023-02-01 api version

tombuildsstuff
tombuildsstuff reviewed Mar 20, 2023
View reviewed changes
internal/services/signalr/signalr_service_resource.go Outdated
@@ -600,16 +693,31 @@ func resourceArmSignalRServiceSchema() map[string]*pluginsdk.Schema {
},
},

"connectivity_logs_enabled": {
Type: pluginsdk.TypeBool,
"resource_logs": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than introducing a breaking change, we can instead make the new field http_request_logs_enabled a top level field and remove this breaking change?

tombuildsstuff
tombuildsstuff suggested changes Mar 20, 2023
View reviewed changes
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.

Looks like we can remove the breaking change here by making the new field http_request_logs_enabled a top-level field like the other two are today?

@xiaxyi xiaxyi changed the title azurerm_signalr_service - adding resource_logs property azurerm_signalr_service - adding http_request_logs_enabled property Mar 21, 2023
@github-actions github-actions bot added size/XL and removed size/L labels Mar 21, 2023
@xiaxyi
Copy link
Contributor Author

xiaxyi commented Mar 21, 2023 •
edited
Loading

Thanks @tombuildsstuff for the comments, I was thinking about the same thing at the first place but turned to introduce the resource_logs block at the end as I would like to keep things the same as live_trace. Anyway, I updated the code as suggested, feel free to let me know if there is anything needed.

@xiaxyi
Copy link
Contributor Author

xiaxyi commented Mar 21, 2023

update acctest result:
--- PASS: TestAccSignalRService_resourceLogs (537.52s)
PASS
ok github.com/hashicorp/terraform-provider-azurerm/internal/services/signalr 538.458s

@xiaxyi xiaxyi changed the title azurerm_signalr_service - adding http_request_logs_enabled property azurerm_signalr_service - add http_request_logs_enabled property Mar 22, 2023
katbyte
katbyte approved these changes Apr 1, 2023
View reviewed changes
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 87bee8d into hashicorp:main Apr 1, 2023
@github-actions github-actions bot added this to the v3.51.0 milestone Apr 1, 2023
katbyte added a commit that referenced this pull request Apr 1, 2023
@katbyte
Update CHANGELOG.md #21032
ac610ca
@github-actions
Copy link

This functionality has been released in v3.51.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 github-actions bot locked as resolved and limited conversation to collaborators Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tombuildsstuff tombuildsstuff tombuildsstuff requested changes

@katbyte katbyte katbyte approved these changes

Assignees
No one assigned
Labels
documentation service/signalr size/XL
Projects
None yet
Milestone
v3.51.0
Development

Successfully merging this pull request may close these issues.
3 participants
@xiaxyi @tombuildsstuff @katbyte