-
Notifications
You must be signed in to change notification settings - Fork 98
tfsdk: Introduce new ProviderServer functions, deprecate existing #294
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
Reference: #22 Provider developers have a few reasons to directly need a tfprotov6.ProviderServer implementation: - When implementing acceptance testing with terraform-plugin-sdk helper/resource. - When using terraform-plugin-mux tf6muxserver.NewMuxServer(). - When using terraform-plugin-mux tf6to5server.DowngradeServer(). The current `NewProtocol6Server(Provider)` function enables these use cases, however the implementation is less than ergonomic for provider developers as a wrapping function is required in all cases: ```go // helper/resource acceptance testing resource.TestCase{ ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ "example": func() (tfprotov6.ProviderServer, error) { return tfsdk.NewProtocol6Server(New()), nil }, }, // ... } // tf6muxserver.NewMuxServer() providers := []func() tfprotov6.ProviderServer{ func() tfprotov6.ProviderServer { return tfsdk.NewProtocol6Server(provider1.New()) }, func() tfprotov6.ProviderServer { return tfsdk.NewProtocol6Server(provider2.New()) }, } muxServer, err := tf6muxserver.NewMuxServer(ctx, providers...) // tf6to5server.DowngradeServer() downgradeServer, err := tf6to5server.DowngradeServer(ctx, func() tfprotov6.ProviderServer { return tfsdk.NewProtocol6Server(provider.New()) }) ``` The new functions simplify these implementations and get provider developers closer to not directly importing terraform-plugin-go (with a few other targeted changes, such as type aliases), e.g. ```go // helper/resource acceptance testing resource.TestCase{ ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ "example": tfsdk.NewProtocol6ProviderServerWithError(New()), }, // ... } // tf6muxserver.NewMuxServer() providers := []func() tfprotov6.ProviderServer{ tfsdk.NewProtocol6ProviderServer(provider1.New()), tfsdk.NewProtocol6ProviderServer(provider2.New()), } muxServer, err := tf6muxserver.NewMuxServer(ctx, providers...) // tf6to5server.DowngradeServer() downgradeServer, err := tf6to5server.DowngradeServer(ctx, tfsdk.NewProtocol6ProviderServer(provider.New())) ``` This change prefers deprecation over straight replacement to give some lead time for various documentation updates across multiple projects. The naming is less than ideal, however it feels necessary to remain generic instead of picking any particular details related to their current usage (e.g. "NewTestServer") as this can/will change over time. The "Protocol6" naming is important should a new major protocol version 7 be released, which this framework must support.
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.
LGTM
// Deprecated: Use NewProtocol6ProviderServer instead. This will be removed | ||
// in an upcoming minor version before v1.0.0. |
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 was going to ask if we really need to do this deprecation phase, but I did a quick GitHub global code search, and while it's in the order of ~70 usages, and it includes the HashiCorp repos, it's still 70 usages.
Yep, deprecation first it is.
Associated terraform.io update: hashicorp/terraform-website#2256 |
Reference: #215 Reference: #294 Reference: #296 This change represents the first half of the work necessary to extract the `tfprotov6.ProviderServer` implementation and helper functions out of the `tfsdk` package and into separate packages. The `providerserver` package will be the provider developer facing functionality, while the next iteration of this refactoring will move the actual server implementation into a separate internal package. Once in that separate internal package, efforts can be made to make that code handle terraform-plugin-go type conversions "at the edge" better.
#308) Reference: #215 Reference: #294 Reference: #296 This change represents the first half of the work necessary to extract the `tfprotov6.ProviderServer` implementation and helper functions out of the `tfsdk` package and into separate packages. The `providerserver` package will be the provider developer facing functionality, while the next iteration of this refactoring will move the actual server implementation into a separate internal package. Once in that separate internal package, efforts can be made to make that code handle terraform-plugin-go type conversions "at the edge" better.
… package functions (#2256) Reference: hashicorp/terraform-plugin-framework#294 Reference: hashicorp/terraform-plugin-framework#308
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 #22 (its technically solved already, this is just cherry on top)
Provider developers have a few reasons to directly need a
tfprotov6.ProviderServer
implementation:The current
NewProtocol6Server(Provider)
function enables these use cases, however the implementation is less than ergonomic for provider developers as a wrapping function is required in all cases:The new functions simplify these implementations and get provider developers closer to not directly importing terraform-plugin-go (with a few other targeted changes in other projects, such as type aliases), e.g.
This change prefers deprecation over straight replacement to give some lead time for various documentation updates across multiple projects.
The naming is less than ideal, however it feels necessary to remain generic instead of picking any particular details related to their current usage (e.g. "NewTestServer") as this can/will change over time. The "Protocol6" naming is important should a new major protocol version 7 be released, which this framework must support.