Skip to content

Conversation

@paddycarver
Copy link
Contributor

Wanted to capture some ideas and see what an interface might look like,
so I started sketching out a new attribute path package.

@paddycarver
Copy link
Contributor Author

I think we want to separate out the concept of an AttributeMatcher and an AttributePath, which this conflates.

AttributePath is good for specifying a single, concrete attribute. It's used in diagnostics and to surface context to provider developers during validate and plan modification, for example.

AttributeMatcher is good for specifying the types of attributes something works on, and is what the Wild* methods in this implementation are trying to achieve. It's useful in things like validation, to be able to say "don't allow this to be set when any other attribute is set", etc.

I think separating those out into separate types/concepts would be helpful in keeping straight which we're talking about, as the above technically would allow provider developers to return wildcards as part of diagnostics, which we'd need to catch and throw an error on because Terraform doesn't support that. We'd also be at risk of accidentally passing wildcard paths to provider developers as part of validation, which is less than helpful.

@paddycarver
Copy link
Contributor Author

AttributePaths also likely want a way to be navigated. For example, a Parent() method that returns the parent of the attribute that the AttributePath is pointing to is something that is useful in most situations. A ChildOf method that takes an AttributePath and returns whether the AttributePath it's being called on begins with AttributePath or not would also likely be useful.

Paddy Carver added 2 commits December 15, 2021 01:28
Wanted to capture some ideas and see what an interface might look like,
so I started sketching out a new attribute path package.
Split paths and the wildcard version out into two different
abstractions. Move them into the `attr` package.
Paddy Carver added 3 commits December 15, 2021 03:12
Add ChildOf method on Path that returns true of a Path contains another
Path as a prefix.

Add String method on Path that returns human-friendly Path
representations. This required adding String methods to our attr.Value
implementations so that we could call them when using elementKeyValues.
This breaks support for sets, but whatever.
@paddycarver paddycarver mentioned this pull request Dec 20, 2021
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@bflad
Copy link
Contributor

bflad commented Jun 22, 2022

It's only taken a few months of refactoring to catch up to this 😂 but we finally got to a place where we wanted with native attribute path handling: #390

Next up is migrating Config/Plan/State.Raw and separately on path expression handling!

@bflad bflad closed this Jun 22, 2022
@bflad bflad deleted the paddy_attr_path branch June 22, 2022 19:07
@github-actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants