-
Notifications
You must be signed in to change notification settings - Fork 35
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
Trim main module outputs #174
Trim main module outputs #174
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.
LGTM. Lmk which PR would be easier to handle merge conflict so can merge one before the other, if you have a preference
description = "The Keyring full name for the KMS Customer Managed Encryption Keys." | ||
value = module.data_governance.cmek_keyring_full_name | ||
|
||
depends_on = [ |
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.
Can we also get rid of other time_sleep.wait_for_bridge_propagation
except for one or two key outputs that users will use in subsequent configs?
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.
I removed the depends_on
from the outputs not explicitly used by the examples.
Should we add some kind of note to the user regarding the propagation issue?
@bharathkkb
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.
@daniel-cit users will likely use it in same pattern as examples. We can add additional based on feedback
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.
the depends_on updates lgtm
Fixes #148