-
Notifications
You must be signed in to change notification settings - Fork 68
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
Question: Is showing a Required field as Optional when setting DefaultFunc the best approach? #42
Comments
I recently updated my tfplugindocs and now encountered this. I find this behavior confusing as well and I agree with all your points. |
It would be great if it skips marking the schema as Optional when |
I looked into this and I think this is not something that can be fixed in "api_token": {
Type: schema.TypeString,
Required: true,
DefaultFunc: schema.EnvDefaultFunc("NETBOX_API_TOKEN", nil),
Description: "Netbox API authentication token. Can be set via the `NETBOX_API_TOKEN` environment variable.",
}, leads to this in the schema (shown with "api_token": {
"type": "string",
"description": "Netbox API authentication token. Can be set via the `NETBOX_API_TOKEN` environment variable.",
"description_kind": "markdown",
"optional": true
}, Note that we went from |
Hello 👋🏻
I'm opening this more for a general discussion regarding the use of
DefaultFunc
with a required field.I've noticed that when adding
DefaultFunc
to an otherwise required field it's rendered in the documentation as 'optional'.Now this make sense because if the value isn't provided by the user in their configuration, then we'll use
schema.EnvDefaultFunc
to get a value (otherwise we return an empty string), but it's also a little bit confusing when you look at the code and seeRequired: true
compared to the documentation saying it's 'optional'.I'm not sure if it's better to still have this field be shown as 'required' in the documentation because ultimately if the user doesn't provide a value (not in config, nor in the environment var) then we'll end up setting an empty string which will (in our case definitely) cause errors. I'd personally still prefer to see this field under "Required" to be explicit that regardless of the use of
DefaultFunc
the user should pay special attention to it.Thoughts?
The text was updated successfully, but these errors were encountered: