Skip to content
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

Implement State, State.Get, State.GetAttribute, Config, Plan #46

Merged
merged 9 commits into from
Jun 10, 2021
Merged

Conversation

kmoe
Copy link
Member

@kmoe kmoe commented Jun 8, 2021

The changes to internal/reflect on main made rather a mess of katystate3. We're up to katystate5 now.

The main work here is the addition of ApplyTerraform5AttributePathStep to all schema types, implementing the tftypes.AttributePathStepper interface so that we can traverse the schema with tftypes.WalkAttributePath.

schema/nested_attributes.go Outdated Show resolved Hide resolved
schema/nested_attributes.go Outdated Show resolved Hide resolved
schema/nested_attributes.go Outdated Show resolved Hide resolved
schema/nested_attributes.go Outdated Show resolved Hide resolved
schema/nested_attributes.go Outdated Show resolved Hide resolved
state_test.go Outdated Show resolved Hide resolved
types/list.go Show resolved Hide resolved
@kmoe
Copy link
Member Author

kmoe commented Jun 9, 2021

It turns out that nested attributes did not need the AttributePathStepper implementation, at least if we want to represent (for example) ListNestedAttributes as a types.ListType. Instead, the schema and its nested attribute types get a recursive AttributeType() method, which returns a reasonable attr.Type representation. This is needed in any case for State.Get.

Please take a look at the tests to see if this provides the appropriate UX.

@kmoe kmoe requested a review from paddycarver June 9, 2021 15:10
@kmoe kmoe changed the title Implement State, State.Get, State.GetAttribute Implement State, State.Get, State.GetAttribute, Config, Plan Jun 9, 2021
Copy link
Contributor

@paddycarver paddycarver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor feedback on this one. I think the only part that's super important is the object using the wrong attribute path step type.

config.go Outdated Show resolved Hide resolved
types/object.go Outdated Show resolved Hide resolved
types/object.go Outdated Show resolved Hide resolved
@kmoe kmoe requested a review from paddycarver June 10, 2021 11:09
Copy link
Contributor

@paddycarver paddycarver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do it. 🚀

@kmoe kmoe merged commit 3e880e4 into main Jun 10, 2021
@kmoe kmoe deleted the katystate5 branch June 10, 2021 11:25
kmoe added a commit that referenced this pull request Jun 11, 2021
paddycarver pushed a commit that referenced this pull request Jun 12, 2021
* Add changelog entry for #44

* Add changelog entry for #46
@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 11, 2021
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.

2 participants