tfsdk: Migrate schema handling to internal interfaces#438
Merged
Conversation
4724394 to
2bb4b7d
Compare
Contributor
Author
|
golangci-lint fixes: #439 |
Reference: #31 Reference: #132 Reference: #223 Reference: #326 Reference: #365 Reference: #389 The main goals of this change are: - Prepare the Go module to support multiple packages that implement concept-specific schema declarations, in particular the `datasource`, `provider`, and `resource` packages - Continue supporting the existing `tfsdk` package schema implementation with minimal provider developer breaking changes, allowing a deprecation period when the concept-specific schemas are introduced - Migrate unexported or unintentially exported `tfsdk` schema functionality to internal packages These goals are accomplished by creating new `internal/fwschema` and `internal/fwxschema` packages, which contain interface types that the provider developer facing packages implement. Currently, the `tfsdk` package is the only implementation, until the design of those concept-specific schema declarations is fully determined. Those designs may include changes to schema implementation details, which cannot be accomplished in the existing `tfsdk` implementation without breaking it and provider developers migrating code to the new packages is a great time to make those types of changes. The internal interface method design here is purposefully similar to the existing `tfsdk` implementations as we cannot name the methods the same as existing fields and to reduce review burden. After the `tfsdk` implementations are removed, we can consider tidying up the interface methods to drop the `Get` and `Is` method name prefixes. There are some minor followup changes that will happen either between or during concept-specific schema implementation work, namely: - Swapping calls `tfsdk` package type `TerraformType()` methods for `AttributeType().TerraformType()` to reduce implementation work for new schema implementations - Updating unit testing that relies on `tfsdk.Schema` to set up sub-tests via `(*testing.T).Run()` to against multiple implementations These were not included here to reduce the review burden as they are separate details which can be handled later.
2bb4b7d to
35ad1e5
Compare
Contributor
Author
|
Thank you for the quick review, @bendbennett -- I think this will be the last v0.11.0 breaking change for refactoring, since there's already a few things queued up. I'll try to get to the two followup items I mention above sometime soon so our v0.12.0 can focus on the actual concept-specific schema design and the enhancements we can do with those. |
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference: #31
Reference: #132
Reference: #223
Reference: #326
Reference: #365
Reference: #389
The main goals of this change are:
datasource,provider, andresourcepackagestfsdkpackage schema implementation with minimal provider developer breaking changes, allowing a deprecation period when the concept-specific schemas are introducedtfsdkschema functionality to internal packagesThese goals are accomplished by creating new
internal/fwschemaandinternal/fwxschemapackages, which contain interface types that the provider developer facing packages implement. Currently, thetfsdkpackage is the only implementation, until the design of those concept-specific schema declarations is fully determined. Those designs may include changes to schema implementation details, which cannot be accomplished in the existingtfsdkimplementation without breaking it and provider developers migrating code to the new packages is a great time to make those types of changes.The internal interface method design here is purposefully similar to the existing
tfsdkimplementations as we cannot name the methods the same as existing fields and to reduce review burden. After thetfsdkimplementations are removed, we can consider tidying up the interface methods to drop theGetandIsmethod name prefixes.There are some minor followup changes that will happen either between or during concept-specific schema implementation work, namely:
tfsdkpackage typeTerraformType()methods forAttributeType().TerraformType()to reduce implementation work for new schema implementationstfsdk.Schemato set up sub-tests via(*testing.T).Run()to against multiple implementationsThese were not included here to reduce the review burden as they are separate details which can be handled later.