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

tfsdk: Migrate schema handling to internal interfaces #438

Merged
merged 2 commits into from
Aug 5, 2022

Conversation

bflad
Copy link
Member

@bflad bflad commented Aug 5, 2022

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.

@bflad bflad added breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. tech-debt Issues tracking technical debt that we're carrying. labels Aug 5, 2022
@bflad bflad requested a review from a team as a code owner August 5, 2022 14:48
@bflad bflad force-pushed the bflad/schema-refactoring branch 2 times, most recently from 4724394 to 2bb4b7d Compare August 5, 2022 15:26
@bflad
Copy link
Member Author

bflad commented Aug 5, 2022

golangci-lint fixes: #439

Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

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

Wow! Nice job.

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.
@bflad bflad added this to the v0.11.0 milestone Aug 5, 2022
@bflad
Copy link
Member Author

bflad commented Aug 5, 2022

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.

@bflad bflad merged commit e6bc60e into main Aug 5, 2022
@bflad bflad deleted the bflad/schema-refactoring branch August 5, 2022 16:55
@github-actions
Copy link

github-actions bot commented Sep 5, 2022

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 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. tech-debt Issues tracking technical debt that we're carrying.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants