-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
configs: explicitly nullable variable values #29832
Conversation
The current behavior of module input variables is to allow users to override a default by assigning `null`, which works contrary to the behavior of resource attributes, and prevents explicitly accepting a default when the input must be defined in the configuration. Add a new variable attribute called `nullable` will allow explicitly defining when a variable can be set to null or not. The current default behavior is that of `nullable=true`. Setting `nullable=false` in a variable block indicates that the variable value can never be null. This either requires a non-null input value, or a non-null default value. In the case of the latter, we also opt-in to the new behavior of a `null` input value taking the default rather than overriding it. In a future language edition where we make `nullable=false` the default, setting `nullable=true` will allow the legacy behavior of `null` overriding a default value. The only future configuration in which this would be required even if the legacy behavior were not desired is when setting an optional+nullable value. In that case `default=null` would also be needed and we could therefor imply `nullable=true` without requiring it in the configuration.
2a34f33
to
f0a64eb
Compare
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 didn't have time yet to read through all of this because I have a meeting shortly but I wanted to just share the one comment I already left inline here, in case it's helpful in the meantime, and then I'll come back and read more of this a bit later if someone else doesn't get there first!
What I read so far looks good, though!
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 like this revised design a lot! There was one part inline that I'm a bit confused by. I think we'll need a docs update for this too.
a0a3908
to
3fed9e8
Compare
3fed9e8
to
7b7972a
Compare
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.
Looks good to me!
I'll follow up with a separate PR for docs |
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. |
The current behavior of module input variables is to allow users to
override a default by assigning
null
, which works contrary to thebehavior of resource attributes, and prevents explicitly accepting a
default when the input must be defined in the configuration.
Add a new variable attribute called
nullable
which will allow explicitlydefining when a variable can be set to null or not. The current default
behavior is that of
nullable=true
.Setting
nullable=false
in a variable block indicates that the variablevalue can never be null. This either requires a non-null input value, or
a non-null default value. In the case of the latter, we also opt-in to
the new behavior of a
null
input value taking the default rather thanoverriding it.
In a future language edition where we make
nullable=false
the default,setting
nullable=true
will allow the legacy behavior ofnull
overriding a default value. The only future configuration in which this
would be required even if the legacy behavior were not desired is when
setting an optional+nullable value. In that case
default=null
wouldalso be needed and we could therefor imply
nullable=true
withoutrequiring it in the configuration.
The possible combinations of variable configuration are:
Fixes #24142