-
Notifications
You must be signed in to change notification settings - Fork 23
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
fix: get availability zone from data source #137
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.
Thanks for the contribution @ugmuka. Minor suggestions (as of the linter warnings).
Co-authored-by: Martin Gerlach <[email protected]> Signed-off-by: ugmuka <[email protected]>
Co-authored-by: Martin Gerlach <[email protected]> Signed-off-by: ugmuka <[email protected]>
Thanks, I signed off your suggestion. |
Ok. Checks are looking good now. Two more things to consider: Is it safe that the AZs are always returned in the same (alphabetical?) order by the data source? And do we need to exclude local zones (see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/availability_zones.html and look for the example "Only availability zones (no local zones)") ? Just want to make sure that users do not get unwanted changes due to changes of the AZs returned by the data source. I am on a short leave from work these days, checking only occasionally. Back on Wednesday. My colleague Jakob @jdiebold may be available Mon/Tue. |
@ugmuka just realized that my second question is not relevant as you used the group-name filter. Regarding the first question, order of AZs, I'll make a code suggestion. |
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.
Suggesting to sort the az names retrieved from the data source.
Also, could you please sync your fork with idealo:main?
Hi @ugmuka , would my suggestions be ok? If yes, please apply them and rebase. Thx. |
Co-authored-by: Martin Gerlach <[email protected]> Signed-off-by: ugmuka <[email protected]>
Co-authored-by: Martin Gerlach <[email protected]> Signed-off-by: ugmuka <[email protected]>
Co-authored-by: Martin Gerlach <[email protected]> Signed-off-by: ugmuka <[email protected]>
Sorry for the late reply, I signed off your commit. |
🎉 This PR is included in version 3.0.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What does this PR do?
When creating subnet, availability zone is hard coded. Fixed to get availability zone parameters from data source.
Motivation
In certain region,
<region>-b
az is not available(exap-northeast-1b
).More
pre-commit run -a
with this PRFor Moderators
Additional Notes