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.

Sign up for GitHub

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

Jump to bottom

Implement Resource Private State Management #433

Merged
merged 28 commits into from
Aug 11, 2022
Merged

Implement Resource Private State Management #433

merged 28 commits into from
Aug 11, 2022

Conversation

bendbennett
Copy link
Contributor

Closes: #399

@bendbennett bendbennett added the enhancement New feature or request label Aug 1, 2022
@bflad bflad self-assigned this Aug 1, 2022
@bflad bflad added this to the v0.11.0 milestone Aug 1, 2022
bflad
bflad reviewed Aug 1, 2022
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Great start! I'm not sure if you're interested in trying the next steps using this new package or potentially waiting until #432 is a little more sorted since it'll be moving the types where this data will wind up. Whatever works for me (if you'd rather do the bulk of the other work with an understanding you may need to rebase for the tfsdk package changes). 😄

internal/privatestate/data.go Outdated Show resolved Hide resolved
internal/privatestate/data.go Outdated Show resolved Hide resolved
internal/privatestate/data.go Outdated Show resolved Hide resolved
internal/privatestate/data.go Outdated Show resolved Hide resolved
internal/privatestate/data_test.go Show resolved Hide resolved
internal/privatestate/data.go Show resolved Hide resolved
bflad
bflad reviewed Aug 1, 2022
View reviewed changes
internal/privatestate/data.go Outdated Show resolved Hide resolved
bflad
bflad reviewed Aug 2, 2022
View reviewed changes
internal/privatestate/data_test.go Show resolved Hide resolved
internal/privatestate/data.go Outdated Show resolved Hide resolved
internal/privatestate/data.go Outdated Show resolved Hide resolved
internal/privatestate/data.go Outdated Show resolved Hide resolved
bflad
bflad reviewed Aug 3, 2022
View reviewed changes
internal/proto5server/server_readresource_test.go Outdated Show resolved Hide resolved
internal/proto5server/server_readresource_test.go Show resolved Hide resolved
internal/proto6server/server_readresource_test.go Show resolved Hide resolved
internal/proto6server/server_readresource_test.go Outdated Show resolved Hide resolved
internal/fwserver/server_readresource.go Show resolved Hide resolved
resource/read.go Outdated Show resolved Hide resolved
@bendbennett bendbennett force-pushed the bendbennett/issues-399 branch 2 times, most recently from 887bb1b to b9ee558 Compare August 8, 2022 15:49
bendbennett and others added 17 commits August 9, 2022 21:48
@bendbennett
Add privatestate pkg and Data type (#399)
8577ca7 
Reference: #399
@bendbennett
Fixing map merging and tests. Adding doc (#399)
d642544 
Reference: #399
@bendbennett
Amend diagnostic messages and add logging (#399)
ca016ec 
Reference: #399
@bendbennett
Setting up private data for use in ReadResource RPC (#399)
129c3c7 
Reference: #399
@bendbennett
Refactoring and fixing tests (#399)
d708e6b 
Reference: #399
@bendbennett
Call privatestate.NewData directly from fromproto5/6 (#399)
4f0e917 
Reference: #399
@bendbennett
Adding tests for proto5/6server private (#399)
b449d82 
Reference: #399
@bendbennett
Additional tests for privatestate.NewData (#399)
007f352 
Reference: #399
@bendbennett
Additional tests for privatestate (#399)
48e978f 
Reference: #399
@bendbennett
Calling Data.Bytes directly from toproto5/6/readresource (#399)
0d591b9 
Reference: #399
@bendbennett
Adding test coverage to verify SetKey behaviour (#399)
a551fce 
Reference: #399
@bendbennett
Adding test coverage for fwserver readresource behaviour (#399)
39de02f 
Reference: #399
@bendbennett @bflad
Apply suggestions from code review
f07b8ed 
Co-authored-by: Brian Flad <[email protected]>
@bendbennett
Adding test coverage for proto5/6server readresource behaviour (#399)
e541dde 
Reference: #399
@bendbennett @bflad
Update resource/read.go
d33ea84 
Co-authored-by: Brian Flad <[email protected]>
@bendbennett
Configuring private state data for use with create, update and delete…
650f26d 
… (applyresourcechange) operations (#399)

Reference: #399
@bendbennett
Refactoring Data to use a pointer to a ProviderData struct which hold…
d39330a 
…s an unexported data field containing map[string][]byte to prevent direct manipulation of the data and inadvertent addition of values that are not invalid (i.e., invalid JSON and/or UTF-8) (#399)

Reference: #399
@bendbennett bendbennett marked this pull request as ready for review August 9, 2022 20:51
@bendbennett bendbennett requested a review from a team as a code owner August 9, 2022 20:51
bflad
bflad reviewed Aug 9, 2022
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Overall this is looking really good, just a few minor things we can tidy up. If you throw in a changelog file, I can review that too. Thanks!

internal/fromproto5/readresource_test.go Outdated Show resolved Hide resolved
internal/fwserver/attribute_plan_modification.go Outdated Show resolved Hide resolved
internal/fwserver/server_applyresourcechange_test.go Outdated Show resolved Hide resolved
internal/fwserver/server_updateresource.go Outdated Show resolved Hide resolved
resource/modify_plan.go Outdated Show resolved Hide resolved
resource/modify_plan.go Outdated Show resolved Hide resolved
resource/update.go Outdated Show resolved Hide resolved
tfsdk/attribute_plan_modification.go Outdated Show resolved Hide resolved
bflad
bflad reviewed Aug 10, 2022
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Almost there! Those new Empty/Must functions make things so much easier to read, nice!

internal/fwserver/attribute_plan_modification.go Outdated Show resolved Hide resolved
internal/fwserver/block_plan_modification.go Outdated Show resolved Hide resolved
internal/fwserver/attribute_plan_modification.go Outdated Show resolved Hide resolved
internal/fwserver/server_planresourcechange.go Outdated Show resolved Hide resolved
internal/fwserver/server_planresourcechange.go Outdated Show resolved Hide resolved
internal/privatestate/data.go Outdated Show resolved Hide resolved
resource/update.go Outdated Show resolved Hide resolved
@bendbennett bendbennett mentioned this pull request Aug 11, 2022
Closed
bflad
bflad approved these changes Aug 11, 2022
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Looks good to me 🚀

@bendbennett bendbennett merged commit 9e664c7 into main Aug 11, 2022
@bendbennett bendbennett deleted the bendbennett/issues-399 branch August 11, 2022 15:33
iwarapter pushed a commit to iwarapter/terraform-plugin-framework that referenced this pull request Sep 4, 2022
@bflad
Update CHANGELOG for hashicorp#433
e8f8d09
@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 Sep 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bflad bflad bflad approved these changes

Assignees

@bflad bflad

Labels
enhancement New feature or request
Projects
None yet
Milestone
v0.11.0
Development

Successfully merging this pull request may close these issues.

Resource Private State Management
2 participants
@bendbennett @bflad