Skip to content

refactor(aws_lb): replace lookups with null checks for optional fields #66

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

Merged

Conversation

nickreynke
Copy link
Contributor

Replaced usage of lookup() with direct null checks using ternary operators for optional fields in various AWS load balancer resources. This improves code readability, consistency, and aligns with Terraform best practices. There are no changes to functional behavior, ensuring backward compatibility.

Replaced usage of lookup() with direct null checks using ternary operators for optional fields in various AWS load balancer resources. This improves code readability, consistency, and aligns with Terraform best practices. There are no changes to functional behavior, ensuring backward compatibility.
@nickreynke
Copy link
Contributor Author

lookups are a good way to use a different value if the corresponding attribute is not found. But this does not work on variables that are built upon a type whose fields may consist of optional(...). If they are not defined, and do not have a default value, they result in being null thus making a lookup succeed because it is "set". So I had to do some null-checking :)

@nickreynke nickreynke marked this pull request as draft April 3, 2025 06:55
Simplified null handling by replacing conditional checks with coalesce function for various target group and health check attributes. This improves readability and aligns with best practices for Terraform expressions. No functional changes are introduced with this update.
@nickreynke nickreynke marked this pull request as ready for review April 3, 2025 07:07
@nickreynke nickreynke marked this pull request as draft April 3, 2025 07:20
@nickreynke nickreynke marked this pull request as ready for review April 3, 2025 07:21
@jnonino jnonino merged commit 694a7f4 into cn-terraform:main Apr 3, 2025
3 checks passed
@nickreynke
Copy link
Contributor Author

Thank you @jnonino!! :) Could you release version 1.0.36 of this module?

@jnonino
Copy link
Member

jnonino commented Apr 3, 2025

Yes, but this was raised after the previous release: #67?
Is that something you can fix before releasing 1.0.36?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants