-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
d/aws_efs_file_system: Document 'file_system_id' attribute #39751
base: main
Are you sure you want to change the base?
d/aws_efs_file_system: Document 'file_system_id' attribute #39751
Conversation
Community NoteVoting for Prioritization
For Submitters
|
@@ -46,6 +46,7 @@ This data source exports the following attributes in addition to the arguments a | |||
* `availability_zone_id` - The identifier of the Availability Zone in which the file system's One Zone storage classes exist. | |||
* `dns_name` - DNS name for the filesystem per [documented convention](http://docs.aws.amazon.com/efs/latest/ug/mounting-fs-mount-cmd-dns-name.html). | |||
* `encrypted` - Whether EFS is encrypted. | |||
* `file_system_id` - The ID that identifies the file system (e.g., fs-ccfc0d65). |
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.
You know, as I re-read these docs, I see above that it says This data source exports the following attributes in addition to the arguments above:
.
Some AWS resources/data-sources fully lay out all attributes in the Attribute Reference section - example. Maybe what would be more useful is a removal of the statement This data source exports the following attributes in addition to the arguments above:
and ensure that the Attribute Reference section covers the exhaustive list.
Thoughts?
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.
Hey @acmcelwee 👋 Thank you for taking the time to put this PR together! As you've mentioned, it looks like file_system_id
is indeed documented in the argument section. The pattern our documentation follows is to split arguments (things that you can supply a value to in your configuration) and attributes (read-only values). This holds true in the aws_autoscaling_group
data source that you linked to; you may only supply a value to the name
argument, while the rest are read-only attributes.
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.
@justinretzolk, I'm not sure if I fully follow your guidance here. Are you saying that the preference is for the docs to clearly lay out all arguments and a separate call out for all attributes? Or, are you saying that this current doc follows the current preference of the hashcorp team?
The docs don't currently seem to consistently follow one way or the other. For example, that ASG data source I referenced takes the hard split of arguments and then an exhaustive list of attributes (that duplicates the name
attribute argument that was listed in the arguments section). On the other side of that, it looks like ~1.4k files use the approach of not restating the arguments in the attributes section and just points the reader to the arguments section as additional attributes. I'm not sure if I'm a lone data point, but I will at least say that multiple people on my team did completely overlook the reference to the arguments as attributes that are also available. I'm happy to reshape this PR in whatever way you want to guide it, but I just need to fully understand your thoughts on how you'd like to proceed.
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.
Welcome @acmcelwee 👋
It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.
Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.
Thanks again, and welcome to the community! 😃
Description
This attribute already exists on the data source, but the documentation was simply missing that attribute.
References