From feee6671826dfb6a618a2d9461d012377ad0be4c Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Mon, 26 Aug 2024 21:43:53 +0200 Subject: [PATCH] - --- CONTRIBUTING.md | 15 ++++++--------- .../common/{provider_factory.go => provider.go} | 1 + .../providers/common/{ => tests}/provider_test.go | 2 +- .../providers/common/{ => tests}/test_utils.go | 2 +- internal/providers/internal/provider.go | 10 ++++++++++ internal/providers/pluginfw/provider.go | 10 +++++----- internal/providers/sdkv2/provider.go | 7 +++++-- .../providers/sdkv2/{ => tests}/coverage_test.go | 5 +++-- .../providers/sdkv2/{ => tests}/generate_test.go | 5 +++-- 9 files changed, 35 insertions(+), 22 deletions(-) rename internal/providers/common/{provider_factory.go => provider.go} (93%) rename internal/providers/common/{ => tests}/provider_test.go (99%) rename internal/providers/common/{ => tests}/test_utils.go (99%) create mode 100644 internal/providers/internal/provider.go rename internal/providers/sdkv2/{ => tests}/coverage_test.go (98%) rename internal/providers/sdkv2/{ => tests}/generate_test.go (98%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2e85eb4395..f6a311f04a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -119,15 +119,12 @@ provider_installation { After installing the necessary software for building provider from sources, you should be able to run `make coverage` to run the tests and see the coverage. ## Package organization for Providers -We are migrating the resource from SDKv2 to Plugin Framework provider and hence both of them exist in the codebase. For uniform code convention, readability and development, they are defined under internal directory under root as: -- `common/provider`: This package contains the changes common to both the providers example provider factory and tests - - imports are declared as: - - `pluginframeworkprovider` "github.com/databricks/terraform-provider-databricks/internal/pluginframework/provider" - - `sdkv2provider` "github.com/databricks/terraform-provider-databricks/internal/sdkv2/provider" -- `sdkv2/provider`: This package contains the changes related to only sdkv2 provider and shouldn't depend on pluginframework provider -- `pluginframework/provider`: This package contains the changes related to only plugin framework provider and shouldn't depend on sdkv2 - -Note: Common dependencies for both providers will go into the internal/common/provider package. +We are migrating the resource from SDKv2 to Plugin Framework provider and hence both of them exist in the codebase. For uniform code convention, readability and development, they are organized in the `internal/providers` directory under root as follows: +- `common`: Contains the changes that `depends` on both internal/providers/sdkv2 and internal/providers/pluginfw packages, eg: `GetProviderServer`. +- `internal`: Contains the changes `used by` both internal/providers/sdkv2 and internal/providers/pluginfw packages, eg: `GetProviderName`. +- `pluginfw`: Contains the changes specific to Plugin Framework. This package shouldn't depend on sdkv2 or common. +- `sdkv2`: Contains the changes specific to SDKv2. This package shouldn't depend on pluginfw or common. + ## Debugging diff --git a/internal/providers/common/provider_factory.go b/internal/providers/common/provider.go similarity index 93% rename from internal/providers/common/provider_factory.go rename to internal/providers/common/provider.go index 706abbb903..3fca00b39b 100644 --- a/internal/providers/common/provider_factory.go +++ b/internal/providers/common/provider.go @@ -1,3 +1,4 @@ +// Package common contains the changes that depends on both internal/providers/sdkv2 and internal/providers/pluginfw packages package common import ( diff --git a/internal/providers/common/provider_test.go b/internal/providers/common/tests/provider_test.go similarity index 99% rename from internal/providers/common/provider_test.go rename to internal/providers/common/tests/provider_test.go index d5568f95e0..a880bc8d41 100644 --- a/internal/providers/common/provider_test.go +++ b/internal/providers/common/tests/provider_test.go @@ -1,4 +1,4 @@ -package common +package tests import ( "context" diff --git a/internal/providers/common/test_utils.go b/internal/providers/common/tests/test_utils.go similarity index 99% rename from internal/providers/common/test_utils.go rename to internal/providers/common/tests/test_utils.go index a9e39c09e3..7b58a97b6b 100644 --- a/internal/providers/common/test_utils.go +++ b/internal/providers/common/tests/test_utils.go @@ -1,4 +1,4 @@ -package common +package tests import ( "context" diff --git a/internal/providers/internal/provider.go b/internal/providers/internal/provider.go new file mode 100644 index 0000000000..53ab74ce7a --- /dev/null +++ b/internal/providers/internal/provider.go @@ -0,0 +1,10 @@ +// Package internal contains the changes used by both internal/providers/sdkv2 and internal/providers/pluginfw packages. +// +// Note: This is different from internal/providers/common which contains the changes that *depends* on both: +// internal/providers/sdkv2 and internal/providers/pluginfw packages. Whereas, internal/providers/internal package contains +// the changes *used* by both internal/providers/sdkv2 and internal/providers/pluginfw packages. +package internal + +func GetProviderName() string { + return "databricks-tf-provider" +} diff --git a/internal/providers/pluginfw/provider.go b/internal/providers/pluginfw/provider.go index 643f3cf918..92cc340c38 100644 --- a/internal/providers/pluginfw/provider.go +++ b/internal/providers/pluginfw/provider.go @@ -1,3 +1,6 @@ +// Package pluginfw contains the changes specific to the plugin framework +// +// Note: This shouldn't depend on internal/providers/sdkv2 or internal/providers/common package pluginfw import ( @@ -12,6 +15,7 @@ import ( "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/terraform-provider-databricks/commands" "github.com/databricks/terraform-provider-databricks/common" + "github.com/databricks/terraform-provider-databricks/internal/providers/internal" "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/diag" @@ -23,10 +27,6 @@ import ( "github.com/hashicorp/terraform-plugin-log/tflog" ) -func GetProviderName() string { - return "databricks-tf-provider" -} - func GetDatabricksProviderPluginFramework() provider.Provider { p := &DatabricksProviderPluginFramework{} return p @@ -50,7 +50,7 @@ func (p *DatabricksProviderPluginFramework) Schema(ctx context.Context, req prov } func (p *DatabricksProviderPluginFramework) Metadata(ctx context.Context, req provider.MetadataRequest, resp *provider.MetadataResponse) { - resp.TypeName = GetProviderName() + resp.TypeName = internal.GetProviderName() resp.Version = common.Version() } diff --git a/internal/providers/sdkv2/provider.go b/internal/providers/sdkv2/provider.go index 632a95dce6..7e26dc0b9c 100644 --- a/internal/providers/sdkv2/provider.go +++ b/internal/providers/sdkv2/provider.go @@ -1,3 +1,6 @@ +// Package sdkv2 contains the changes specific to the SDKv2 +// +// Note: This package shouldn't depend on internal/providers/pluginfw or internal/providers/common package sdkv2 import ( @@ -26,7 +29,7 @@ import ( "github.com/databricks/terraform-provider-databricks/commands" "github.com/databricks/terraform-provider-databricks/common" "github.com/databricks/terraform-provider-databricks/dashboards" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw" + "github.com/databricks/terraform-provider-databricks/internal/providers/internal" "github.com/databricks/terraform-provider-databricks/jobs" "github.com/databricks/terraform-provider-databricks/logger" "github.com/databricks/terraform-provider-databricks/mlflow" @@ -51,7 +54,7 @@ import ( func init() { // IMPORTANT: this line cannot be changed, because it's used for // internal purposes at Databricks. - useragent.WithProduct(pluginfw.GetProviderName(), common.Version()) + useragent.WithProduct(internal.GetProviderName(), common.Version()) userAgentExtraEnv := os.Getenv("DATABRICKS_USER_AGENT_EXTRA") out, err := ParseUserAgentExtra(userAgentExtraEnv) diff --git a/internal/providers/sdkv2/coverage_test.go b/internal/providers/sdkv2/tests/coverage_test.go similarity index 98% rename from internal/providers/sdkv2/coverage_test.go rename to internal/providers/sdkv2/tests/coverage_test.go index 469fcb6bb1..a9fff7ff25 100644 --- a/internal/providers/sdkv2/coverage_test.go +++ b/internal/providers/sdkv2/tests/coverage_test.go @@ -1,4 +1,4 @@ -package sdkv2 +package tests import ( "fmt" @@ -12,6 +12,7 @@ import ( "strings" "testing" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/stretchr/testify/assert" ) @@ -143,7 +144,7 @@ func TestCoverageReport(t *testing.T) { files, err := recursiveChildren("..") assert.NoError(t, err) - p := DatabricksProvider() + p := sdkv2.DatabricksProvider() var cr CoverageReport var longestResourceName, longestFieldName int diff --git a/internal/providers/sdkv2/generate_test.go b/internal/providers/sdkv2/tests/generate_test.go similarity index 98% rename from internal/providers/sdkv2/generate_test.go rename to internal/providers/sdkv2/tests/generate_test.go index b2808395f7..4b12513670 100644 --- a/internal/providers/sdkv2/generate_test.go +++ b/internal/providers/sdkv2/tests/generate_test.go @@ -1,4 +1,4 @@ -package sdkv2 +package tests import ( "fmt" @@ -11,6 +11,7 @@ import ( "testing" "text/template" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/stretchr/testify/assert" ) @@ -233,7 +234,7 @@ func TestGenerateTestCodeStubs(t *testing.T) { t.Logf("Got %d unit tests in total. %v", len(funcs), resourceTestStub{}) t.Skip() - p := DatabricksProvider() + p := sdkv2.DatabricksProvider() for name, resource := range p.ResourcesMap { if name != "databricks_group_instance_profile" { continue