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_express_route_circuit_peering - support for bandwidth_in_gbps, express_route_port_id #12289

Merged
merged 18 commits into from
Jun 24, 2021
Merged

azurerm_express_route_circuit_peering - support for bandwidth_in_gbps, express_route_port_id #12289

merged 18 commits into from
Jun 24, 2021

Conversation

xuzhang3
Copy link
Contributor

No description provided.

magodo
magodo requested changes Jun 21, 2021
View reviewed changes
Copy link
Collaborator

@magodo magodo left a comment

Choose a reason for hiding this comment

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

@xuzhang3 Thank you for the PR! I've looked through and left some comments.

azurerm/internal/services/network/express_route_circuit_resource.go Outdated Show resolved Hide resolved
azurerm/internal/services/network/express_route_circuit_resource.go Outdated Show resolved Hide resolved
azurerm/internal/services/network/express_route_circuit_resource.go Outdated Show resolved Hide resolved
azurerm/internal/services/network/express_route_circuit_resource.go Outdated Show resolved Hide resolved
azurerm/internal/services/network/express_route_circuit_resource.go Outdated Show resolved Hide resolved
website/docs/r/express_route_circuit.html.markdown Outdated Show resolved Hide resolved
website/docs/r/express_route_circuit.html.markdown Outdated Show resolved Hide resolved
website/docs/r/express_route_circuit.html.markdown Outdated Show resolved Hide resolved
website/docs/r/express_route_circuit.html.markdown Outdated Show resolved Hide resolved
website/docs/r/express_route_circuit.html.markdown Outdated Show resolved Hide resolved
@github-actions github-actions bot added size/L and removed size/XL labels Jun 22, 2021
magodo
magodo requested changes Jun 22, 2021
View reviewed changes
Copy link
Collaborator

@magodo magodo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the update!
I have left some additional mostly minor comments that once addressed this should be good to merge 👍

azurerm/internal/services/network/express_route_circuit_resource.go Outdated Show resolved Hide resolved
website/docs/r/express_route_circuit.html.markdown Outdated Show resolved Hide resolved
website/docs/r/express_route_circuit.html.markdown Outdated Show resolved Hide resolved
@magodo
Copy link
Collaborator

magodo commented Jun 22, 2021

@xuzhang3 overall it LGTM! Despite, the test has one small error, which looks like wants to adding some sequential in the acctest. If we can fix that, it should be good to go!

image

@xuzhang3
Copy link
Contributor Author

@xuzhang3 overall it LGTM! Despite, the test has one small error, which looks like wants to adding some sequential in the acctest. If we can fix that, it should be good to go!

image

Every subscription can create no more than 10 circuits by default. https://docs.microsoft.com/en-us/azure/expressroute/expressroute-faqs#can-i-have-more-than-one-expressroute-circuit-in-my-subscription

@magodo
Copy link
Collaborator

magodo commented Jun 23, 2021

@xuzhang3 So can we reflect that in the acctest?

@github-actions github-actions bot added size/XL and removed size/L labels Jun 23, 2021
@xuzhang3
Copy link
Contributor Author

Fixed

@xuzhang3 So can we reflect that in the acctest?

Fixed

@magodo
Copy link
Collaborator

magodo commented Jun 24, 2021

LGTM now, thank you!
image

katbyte
katbyte approved these changes Jun 24, 2021
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 changed the title Create Express Route Circuits on Express Route Port Create Express Route Circuits on Express Route Port support for bandwidth_in_gbps, express_route_port_id Jun 24, 2021
@katbyte katbyte changed the title Create Express Route Circuits on Express Route Port support for bandwidth_in_gbps, express_route_port_id azurerm_express_route_circuit_peering - support for bandwidth_in_gbps, express_route_port_id Jun 24, 2021
@katbyte katbyte merged commit a3cbbbc into hashicorp:master Jun 24, 2021
katbyte added a commit that referenced this pull request Jun 24, 2021
@katbyte
CHANGELOG.md for #12289
f0c2f15
@bardahlm bardahlm mentioned this pull request Jun 30, 2021
Closed
@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 Jul 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@magodo magodo magodo requested changes

@katbyte katbyte katbyte approved these changes

Assignees
No one assigned
Labels
documentation enhancement service/express-route size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xuzhang3 @magodo @katbyte