-
Notifications
You must be signed in to change notification settings - Fork 98
Document Relationships Between Terraform Commands, Protocol RPCs, and Framework Functionality #618
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
Conversation
…form commands, RPCs and framework functionality (#587)
|
||
### Overview | ||
|
||
 |
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 we have HTML comments (or some other documentation added) for how to update these diagrams?
{ | ||
"title": "Protocol RPCs", | ||
"path": "rpcs" | ||
}, |
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 to raise a slight concern: I'm a little worried that this page is a little too detailed in its current form to be the first potential page of the navigation. The information is real nice (e.g. relating Terraform commands to how they interact with provider code), but I think it might completely overwhelm newer developers with a lot of implementation bits that are not fully necessary for them to successfully develop a provider. I'm not sure I have the best constructive feedback yet on how to address that potential concern though. 🙁
One thing is that we currently have pages for each of the provider implementation details, such as Providers > Validating Configuration. Maybe some of this content could exist towards the bottom of those for advanced readers so its a little more self-contained depending on what a developer is wanting to do. Otherwise, I think we should consider how to link those pages back to this information in some form.
I don't think doing the above would fully alleviate the need for some of the higher level information of this page, but it might help quell the information overload issue by colocating the nitty-gritty RPC details as an "implementation details" section for each of the other pages that has a call out that this information is provided only for developers wanting a more technical deep dive.
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.
Might be best if we discuss how we want to proceed here. I'm more than happy to add information and/or links wherever you think would best serve the reader.
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.
Have refactored following discussion to include a new Internals
section in the documentation and link to the RPCs and Framework Functionality
page from other pages in the docs.
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.
Great work! 👍🏻
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.
A few minor things, otherwise looks good to me 🚀
@@ -51,6 +53,8 @@ func (p *ExampleCloudProvider) Configure(ctx context.Context, req provider.Confi | |||
|
|||
Implement the [`datasource.DataSourceWithConfigure` interface](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/datasource#DataSourceWithConfigure) which receives the provider configured data from the [`Provider` interface `Configure` method](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/provider#Provider.Configure) and saves it into the [`datasource.DataSource` interface](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/datasource#DataSource) implementation. | |||
|
|||
The [`datasource.DataSourceWithConfigure` interface `Configure` method](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/datasource#DataSourceWithConfigure.Configure) is called during execution of the [`terraform validate`](/cli/commands/validate), [`terraform plan`](/cli/commands/plan) and [`terraform apply`](/cli/commands/apply) commands when the [`ValidateDataResourceConfig`](/plugin/framework/internals/rpcs#validatedataresourceconfig-rpc) RPC is sent. Additionally, the [`datasource.DataSourceWithConfigure` interface `Configure` method](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/datasource#DataSourceWithConfigure.Configure) is called during execution of the [`terraform plan`](/cli/commands/plan) and [`terraform apply`](/cli/commands/apply) commands when the [`ReadDataSource`](/plugin/framework/internals/rpcs#readdatasource-rpc) RPC is sent. |
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 think if we are going to call out this level of detail, then we should clarify that Terraform calling the ValidateDataResourceConfig
RPC would not call the ConfigureProvider
RPC first, so implementations need to account for that situation. Configuration validation should be "offline" in Terraform.
"timeouts": timeouts.Attributes(ctx, timeouts.Opts{ | ||
Read: true, | ||
}), | ||
"timeouts": timeouts.Attributes(ctx), |
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.
Nit: Tabs vs spaces
|
||
### Summary | ||
|
||
 |
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.
Should we include code comments for these to the website/img README?
"timeouts": timeouts.Attributes(ctx, timeouts.Opts{ | ||
Read: true, | ||
}), | ||
"timeouts": timeouts.Attributes(ctx), |
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.
Nit: Tabs vs spaces
@@ -63,6 +63,28 @@ configuration. Use muxing when you want to release a version of your provider wi | |||
sources migrated to the Framework. You may want to do this if your provider manages a large number of resources and | |||
data sources. | |||
|
|||
|
|||
~> **Important**: If you are using muxing and your SDK provider schema is using |
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.
Doesn't have to be changed now, but FYI about new callout syntax: https://github.com/hashicorp/terraform-providers-devex-internal/issues/124
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.
Will come back to this one. Using <Highlight>....</Highlight> resulted in backtick highlighting being ignored and output literally.
@@ -51,6 +53,8 @@ func (p *ExampleCloudProvider) Configure(ctx context.Context, req provider.Confi | |||
|
|||
Implement the [`resource.ResourceWithConfigure` interface](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/resource#ResourceWithConfigure) which receives the provider configured data from the [`Provider` interface `Configure` method](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/provider#Provider.Configure) and saves it into the [`resource.Resource` interface](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/resource#Resource) implementation. | |||
|
|||
The [`resource.ResourceWithConfigure` interface `Configure` method](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/resource#ResourceWithConfigure.Configure) is called during execution of the [`terraform validate`](/cli/commands/validate), [`terraform plan`](/cli/commands/plan) and [`terraform apply`](/cli/commands/apply) commands when the [`ValidateResourceConfig`](/plugin/framework/internals/rpcs#validateresourceconfig-rpc) RPC is sent. Additionally, the [`resource.ResourceWithConfigure` interface `Configure` method](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/resource#ResourceWithConfigure.Configure) is called during execution of the [`terraform plan`](/cli/commands/plan) and [`terraform apply`](/cli/commands/apply) commands when the [`ReadResource`](/plugin/framework/internals/rpcs#readresource-rpc) RPC is sent. |
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.
Similarly --
I think if we are going to call out this level of detail, then we should clarify that Terraform calling the ValidateDataResourceConfig
RPC would not call the ConfigureProvider
RPC first, so implementations need to account for that situation. Configuration validation should be "offline" in Terraform.
Co-authored-by: Brian Flad <[email protected]>
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. |
Closes: #587
Closes: #640
Closes: #641
The documentation added in this PR contains links which reference changes in Adding Terraform Plugin Protocol top-level section (#241), which should be merged first to avoid broken links in the Framework documentation.