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_network_packet_capture - support for the target_type and scope properties #19385

Merged
merged 15 commits into from
Dec 15, 2022
Merged

azurerm_network_packet_capture - support for the target_type and scope properties #19385

merged 15 commits into from
Dec 15, 2022

Conversation

neil-yechenwei
Copy link
Contributor

@neil-yechenwei neil-yechenwei commented Nov 22, 2022 •
edited
Loading

This PR is to support new properties target_type and scope.

--- PASS: TestAccNetworkWatcher/PacketCapture/localDisk (535.58s)
--- PASS: TestAccNetworkWatcher/PacketCapture/storageAccount (457.34s)
--- PASS: TestAccNetworkWatcher/PacketCapture/storageAccountAndLocalDisk (525.55s)
--- PASS: TestAccNetworkWatcher/PacketCapture/withFilters (487.18s)
--- PASS: TestAccNetworkWatcher/PacketCapture/requiresImport (524.49s)
--- PASS: TestAccNetworkWatcher/VMPacketCapture/storageAccountAndLocalDisk (501.07s)
--- PASS: TestAccNetworkWatcher/VMPacketCapture/withFilters (499.95s)
--- PASS: TestAccNetworkWatcher/VMPacketCapture/requiresImport (527.61s)
--- PASS: TestAccNetworkWatcher/VMPacketCapture/localDisk (487.54s)
--- PASS: TestAccNetworkWatcher/VMPacketCapture/storageAccount (495.20s)
--- PASS: TestAccNetworkWatcher/VMSSPacketCapture/localDisk (558.91s)
--- PASS: TestAccNetworkWatcher/VMSSPacketCapture/storageAccount (610.47s)
--- PASS: TestAccNetworkWatcher/VMSSPacketCapture/storageAccountAndLocalDisk (579.17s)
--- PASS: TestAccNetworkWatcher/VMSSPacketCapture/withFilters (559.13s)
--- PASS: TestAccNetworkWatcher/VMSSPacketCapture/requiresImport (592.10s)
--- PASS: TestAccNetworkWatcher/VMSSPacketCapture/machineScope (536.78s)

stephybun
stephybun requested changes Nov 29, 2022
View reviewed changes
internal/services/network/network_packet_capture_resource.go Outdated Show resolved Hide resolved
internal/services/network/migration/network_packet_capture.go Outdated Show resolved Hide resolved
internal/services/network/network_packet_capture_resource.go Outdated Show resolved Hide resolved
internal/services/network/network_packet_capture_resource.go Outdated Show resolved Hide resolved
internal/services/network/network_packet_capture_resource.go Outdated Show resolved Hide resolved
internal/services/network/network_packet_capture_resource.go Outdated Show resolved Hide resolved
internal/services/network/network_packet_capture_resource_test.go Outdated Show resolved Hide resolved
internal/services/network/network_packet_capture_resource_test.go Outdated Show resolved Hide resolved
internal/services/network/network_packet_capture_resource_test.go Outdated Show resolved Hide resolved
website/docs/r/network_packet_capture.html.markdown Outdated Show resolved Hide resolved
stephybun
stephybun requested changes Nov 30, 2022
View reviewed changes
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.

This requires some rework as outlined in #19385 (comment)

stephybun
stephybun requested changes Dec 8, 2022
View reviewed changes
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.

Appreciate you splitting this out into separate resources and deprecating the old one @neil-yechenwei. This is off to a good start, there are improvements to be made to the schema though, in addition these also need to be typed resources, since they will need to be in the future anyway.

internal/services/network/network_packet_capture_resource.go Outdated Show resolved Hide resolved
internal/services/network/virtual_machine_packet_capture_resource.go Outdated Show resolved Hide resolved
internal/services/network/virtual_machine_packet_capture_resource_test.go Outdated Show resolved Hide resolved
internal/services/network/virtual_machine_scale_set_packet_capture_resource.go Outdated Show resolved Hide resolved
internal/services/network/virtual_machine_packet_capture_resource.go Outdated Show resolved Hide resolved
website/docs/r/virtual_machine_scale_set_packet_capture.html.markdown Outdated Show resolved Hide resolved
website/docs/r/virtual_machine_scale_set_packet_capture.html.markdown Outdated Show resolved Hide resolved
website/docs/r/virtual_machine_scale_set_packet_capture.html.markdown Outdated Show resolved Hide resolved
website/docs/r/virtual_machine_scale_set_packet_capture.html.markdown Outdated Show resolved Hide resolved
website/docs/r/virtual_machine_scale_set_packet_capture.html.markdown Outdated Show resolved Hide resolved
@neil-yechenwei
Copy link
Contributor Author

Thanks for the comment. I've fixed the historical problems you mentioned. Then I will convert this untyped resource to typed resource.

@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Dec 14, 2022 •
edited
Loading

@stephybun , as converting untyped resource to typed resource is not a simple change and I have to make two new typed resources in this one PR, it may take a lot of time. So does it make sense to merge this PR first and then convert them in the future?

stephybun
stephybun approved these changes Dec 15, 2022
View reviewed changes
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.

Tests look good, thanks @neil-yechenwei LGTM ❄️

@stephybun stephybun merged commit 2bd242c into hashicorp:main Dec 15, 2022
stephybun added a commit that referenced this pull request Dec 15, 2022
@stephybun
Update for #19385
99d4f7a
@stephybun stephybun added this to the v3.36.0 milestone Dec 15, 2022
@github-actions
Copy link

This functionality has been released in v3.36.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!

favoretti pushed a commit to favoretti/terraform-provider-azurerm that referenced this pull request Jan 12, 2023
@neil-yechenwei @favoretti
azurerm_network_packet_capture - support for the target_type and …
94268e4 
…`scope` properties (hashicorp#19385)

* azurerm_network_packet_capture - support for the target_type and scope properties

* azurerm_network_packet_capture - support for the target_type and scope properties

* update code

* update code

* update code

* update code

* update code

* update code

* update code

* update code

* update code

* update code
favoretti pushed a commit to favoretti/terraform-provider-azurerm that referenced this pull request Jan 12, 2023
@stephybun @favoretti
Update for hashicorp#19385
4510d51
@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 Jan 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stephybun stephybun stephybun approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@neil-yechenwei @stephybun