-
Notifications
You must be signed in to change notification settings - Fork 19
Docs/data block #520
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
Docs/data block #520
Conversation
Vercel Previews Deployed
|
@@ -1127,6 +1127,10 @@ | |||
"title": "Check block", | |||
"path": "block/check" | |||
}, | |||
{ | |||
"title": "data", |
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.
General note that should maybe be cleaned up on the roll up PR - these are inconsistent in the reference section. we might want to call everything "X block" for SEO/discoverability. doesnt need to be addressed in this PR, ill let you and rose consider best approach.
also, the header for reference is all caps but others arent
|
||
#### Summary | ||
|
||
- Data type: Number. |
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.
Adding the period at the end of these to our style discussion this week, but fine to merge this PR!
|
||
- Data type: List. | ||
- Default: None. | ||
- Example: [Specify a dependency](#specify-a-dependency). |
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.
thrilled to have examples now. this is great.
| `condition` | Expression that must return `true` for Terraform to perform operations on downstream resources. You can refer to any other object in the same configuration scope unless the reference creates a cyclic dependency. | Expression that can include references, strings, and operators. | | ||
| `error_message` | Message that Terraform prints to the console if the `condition` returns `false`. | String | | ||
|
||
Refer to [Test and validate](/terraform/language/test-and-validate) for information about adding validations to your Terraform configuration. |
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 you move this to the end of the section? makes more sense after you provide details rather than interrupting the description.
resource "aws_instance" "example_instance" { | ||
ami = data.aws_ami.example_ami.id | ||
instance_type = "t2.micro" | ||
subnet_id = aws_subnet.example_subnet.id | ||
} |
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 may need to check me on this, but i believe that bc this config makes the resource dependent on both subnet and ami, this will still have the right dependency ordering even without the depends_on in the data source. just a hunch.
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 find this example really interesting actually. It's valid configuration for sure, and while the depends_on
does change the dependency graph, it doesn't really change it in a meaningful way, it just makes it more serial.
I'm not sure this specific example is realistic, though please correct me if I'm wrong if there's some context I'm not aware of. And to be fair, I'm having trouble thinking of an example that isn't overly complex.
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 really struggled to come up an example. As a quick follow up, I'll ask the SME channel for help with a more realistic example that isn't too complicated. For now, I'll update the description to call out its less-than-realisitic/useful nature.
|
||
The following `data` block includes all built-in arguments supported in a data source: | ||
|
||
```hcl |
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'm not sure if this is something we've discussed, but as-is the spec reference doesn't align the equal signs. I also realize that this isn't something we can just terraform fmt
and is a massive effort to upkeep. I don't particularly hold a strong opinion here, just wanted to flag.
|
||
The `for_each` meta-argument accepts a map or a set of strings and creates an instance for each item in that map or set. Each instance is associated with a distinct infrastructure object. Terraform creates, updates, or destroys each instance when applying changes to the configuration. | ||
|
||
You can use pure functions, such as `toset()` and `tomap()`, to create a map or set for use in the `for_each` argument. Whether iterating over the keys of a map or set of strings, all must be known values. Otherwise, Terraform prints an error message that `for_each` has dependencies that it cannot determine before applying the configuration. |
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.
Just wanted to verify that we want to go this deep in this doc. All good if this is what we previously agreed on, but I see we repeat this in the resource
reference doc as well. Maybe a future-looking improvement, but maybe this would be a good case to capture in a partial?
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.
Just wanted to verify that we want to go this deep in this doc. All good if this is what we previously agreed on, but I see we repeat this in the
resource
reference doc as well. Maybe a future-looking improvement, but maybe this would be a good case to capture in a partial?
The information is almost the same as the resource
block. Ideally, we'd be able to parameterize certain words or phrases in a partial for these types of cases. It's on my list to look into tweaking some of these descriptions so that we can reuse them as partials.
resource "aws_instance" "example_instance" { | ||
ami = data.aws_ami.example_ami.id | ||
instance_type = "t2.micro" | ||
subnet_id = aws_subnet.example_subnet.id | ||
} |
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 find this example really interesting actually. It's valid configuration for sure, and while the depends_on
does change the dependency graph, it doesn't really change it in a meaningful way, it just makes it more serial.
I'm not sure this specific example is realistic, though please correct me if I'm wrong if there's some context I'm not aware of. And to be fair, I'm having trouble thinking of an example that isn't overly complex.
7c6635e
into
docs/reference-rewrites-phrase-1
This PR added the
data
block reference page. This is related to 513