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.

Sign up for GitHub

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

chore: Remove unused old implementation from snowflake pkg #2458

Merged
merged 12 commits into from
Feb 6, 2024
Merged

chore: Remove unused old implementation from snowflake pkg #2458

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7660802
Remove user from snowflake package
sfc-gh-asawicki Feb 1, 2024
3596e94
Remove current user from snowflake package
sfc-gh-asawicki Feb 1, 2024
01ec3a4
Migrate current account datasource
sfc-gh-asawicki Feb 1, 2024
508a019
Migrate resource monitor datasource
sfc-gh-asawicki Feb 2, 2024
4b5d373
Migrate role datasource
sfc-gh-asawicki Feb 2, 2024
32c0376
Remove current account logic from snowflake package
sfc-gh-asawicki Feb 2, 2024
87b27c0
Remove unused logic from snowflake package
sfc-gh-asawicki Feb 2, 2024
887f5e1
Fix linter complaints
sfc-gh-asawicki Feb 2, 2024
004372c
Fix network policy
sfc-gh-asawicki Feb 5, 2024
ce142dc
Add issue number to TODO comment
sfc-gh-asawicki Feb 5, 2024
c445254
Fix after review
sfc-gh-asawicki Feb 6, 2024
8929426
Merge branch 'main' into remove-unused-old-implementation
sfc-gh-asawicki Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions pkg/datasources/current_account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package datasources

import (
"context"
"database/sql"
"fmt"
"log"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

Expand Down Expand Up @@ -40,23 +41,26 @@ func CurrentAccount() *schema.Resource {
// ReadCurrentAccount read the current snowflake account information.
func ReadCurrentAccount(d *schema.ResourceData, meta interface{}) error {
db := meta.(*sql.DB)
acc, err := snowflake.ReadCurrentAccount(db)
ctx := context.Background()
client := sdk.NewClientFromDB(db)

current, err := client.ContextFunctions.Current(ctx)
if err != nil {
log.Println("[DEBUG] current_account failed to decode")
d.SetId("")
return nil
}

d.SetId(fmt.Sprintf("%s.%s", acc.Account, acc.Region))
accountErr := d.Set("account", acc.Account)
d.SetId(fmt.Sprintf("%s.%s", current.Account, current.Region))
accountErr := d.Set("account", current.Account)
if accountErr != nil {
return accountErr
}
regionErr := d.Set("region", acc.Region)
regionErr := d.Set("region", current.Region)
if regionErr != nil {
return regionErr
}
url, err := acc.AccountURL()
url, err := current.AccountURL()
if err != nil {
log.Println("[DEBUG] generating snowflake url failed")
return nil
Expand Down
8 changes: 7 additions & 1 deletion pkg/datasources/current_account_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,18 @@ package datasources_test
import (
"testing"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/tfversion"
)

func TestAcc_CurrentAccount(t *testing.T) {
resource.ParallelTest(t, resource.TestCase{
Providers: providers(),
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: nil,
Steps: []resource.TestStep{
{
Expand Down
13 changes: 8 additions & 5 deletions pkg/datasources/current_role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package datasources

import (
"context"
"database/sql"
"fmt"
"log"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

Expand All @@ -26,15 +26,18 @@ func CurrentRole() *schema.Resource {

func ReadCurrentRole(d *schema.ResourceData, meta interface{}) error {
db := meta.(*sql.DB)
role, err := snowflake.ReadCurrentRole(db)
ctx := context.Background()
client := sdk.NewClientFromDB(db)

role, err := client.ContextFunctions.CurrentRole(ctx)
if err != nil {
log.Printf("[DEBUG] current_role failed to decode")
d.SetId("")
return nil
}

d.SetId(fmt.Sprintf(role.Role))
err = d.Set("name", role.Role)
d.SetId(role)
err = d.Set("name", role)
if err != nil {
return err
}
Expand Down
10 changes: 8 additions & 2 deletions pkg/datasources/current_role_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,18 @@ package datasources_test
import (
"testing"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/tfversion"
)

func TestAcc_CurrentRole(t *testing.T) {
resource.ParallelTest(t, resource.TestCase{
Providers: providers(),
resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
Steps: []resource.TestStep{
{
Config: currentRole(),
Expand Down
35 changes: 15 additions & 20 deletions pkg/datasources/resource_monitors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package datasources

import (
"context"
"database/sql"
"errors"
"fmt"
"log"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

Expand Down Expand Up @@ -49,8 +49,10 @@ func ResourceMonitors() *schema.Resource {

func ReadResourceMonitors(d *schema.ResourceData, meta interface{}) error {
db := meta.(*sql.DB)
client := sdk.NewClientFromDB(db)
ctx := context.Background()

account, err := snowflake.ReadCurrentAccount(db)
account, err := client.ContextFunctions.Current(ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think this function should be named CurrentAccount() https://docs.snowflake.com/en/sql-reference/functions/current_account. Also should this be put in a helper struct so it can be something like client.Context.Sessions.CurrentAccount()? Or maybe just client.ContextSessions.CurrentAccount?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We already have current account check another comment).

I don't understand the second part, though. It's already exposed in client's ContextFunctions interface.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have a proposal for the name other than client.ContextFunctions.SessionContext(). Until we come up with a better name I don't mind staying with Current as only we are the users of the SDK.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed to CurrentSessionDetails (because CurrentSession was already taken too).

if err != nil {
log.Print("[DEBUG] unable to retrieve current account")
d.SetId("")
Expand All @@ -59,29 +61,22 @@ func ReadResourceMonitors(d *schema.ResourceData, meta interface{}) error {

d.SetId(fmt.Sprintf("%s.%s", account.Account, account.Region))

currentResourceMonitors, err := snowflake.ListResourceMonitors(db)
if errors.Is(err, sql.ErrNoRows) {
// If not found, mark resource to be removed from state file during apply or refresh
log.Printf("[DEBUG] no resource monitors found in account (%s)", d.Id())
d.SetId("")
return nil
} else if err != nil {
extractedResourceMonitors, err := client.ResourceMonitors.Show(ctx, &sdk.ShowResourceMonitorOptions{})
if err != nil {
log.Printf("[DEBUG] unable to parse resource monitors in account (%s)", d.Id())
d.SetId("")
return nil
}

resourceMonitors := []map[string]interface{}{}

for _, resourceMonitor := range currentResourceMonitors {
resourceMonitorMap := map[string]interface{}{}

resourceMonitorMap["name"] = resourceMonitor.Name.String
resourceMonitorMap["frequency"] = resourceMonitor.Frequency.String
resourceMonitorMap["credit_quota"] = resourceMonitor.CreditQuota.String
resourceMonitorMap["comment"] = resourceMonitor.Comment.String
resourceMonitors := make([]map[string]any, len(extractedResourceMonitors))

resourceMonitors = append(resourceMonitors, resourceMonitorMap)
for i, resourceMonitor := range extractedResourceMonitors {
resourceMonitors[i] = map[string]any{
"name": resourceMonitor.Name,
"frequency": resourceMonitor.Frequency,
"credit_quota": fmt.Sprintf("%f", resourceMonitor.CreditQuota),
"comment": resourceMonitor.Comment,
}
}

return d.Set("resource_monitors", resourceMonitors)
Expand Down
12 changes: 10 additions & 2 deletions pkg/datasources/resource_monitors_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,22 @@ import (
"strings"
"testing"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"

"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/tfversion"
)

func TestAcc_ResourceMonitors(t *testing.T) {
resourceMonitorName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
resource.ParallelTest(t, resource.TestCase{
Providers: providers(),

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
Copy link
Collaborator

Choose a reason for hiding this comment

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

eventually we should make this a configurable variable that can be set in one place and used everywhere. will allow us to upgrade required tf version as necessary

},
CheckDestroy: nil,
Steps: []resource.TestStep{
{
Expand Down
20 changes: 10 additions & 10 deletions pkg/datasources/role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package datasources

import (
"context"
"database/sql"
"errors"
"log"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

Expand Down Expand Up @@ -37,23 +37,23 @@ func Role() *schema.Resource {
// ReadRole Reads the database metadata information.
func ReadRole(d *schema.ResourceData, meta interface{}) error {
db := meta.(*sql.DB)
client := sdk.NewClientFromDB(db)
ctx := context.Background()

roleName := d.Get("name").(string)
role, err := snowflake.NewRoleBuilder(db, roleName).Show()

if errors.Is(err, sql.ErrNoRows) {
role, err := client.Roles.ShowByID(ctx, sdk.NewShowByIdRoleRequest(sdk.NewAccountObjectIdentifier(roleName)))
if err != nil {
log.Printf("[DEBUG] role (%s) not found", roleName)
d.SetId("")
return nil
}
if err != nil {
return err
}

d.SetId(role.Name.String)
if err := d.Set("name", role.Name.String); err != nil {
d.SetId(role.Name)
if err := d.Set("name", role.Name); err != nil {
return err
}
if err := d.Set("comment", role.Comment.String); err != nil {
if err := d.Set("comment", role.Comment); err != nil {
return err
}
return nil
Expand Down
10 changes: 9 additions & 1 deletion pkg/datasources/role_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,23 @@ import (
"strings"
"testing"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"

"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/tfversion"
)

func TestAcc_Role(t *testing.T) {
roleName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
comment := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))

resource.ParallelTest(t, resource.TestCase{
Providers: providers(),
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: nil,
Steps: []resource.TestStep{
{
Expand Down
12 changes: 10 additions & 2 deletions pkg/datasources/row_access_policies_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,24 @@ import (
"strings"
"testing"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"

"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/tfversion"
)

func TestAcc_RowAccessPolicies(t *testing.T) {
databaseName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
schemaName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
rowAccessPolicyName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
resource.ParallelTest(t, resource.TestCase{
Providers: providers(),

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: nil,
Steps: []resource.TestStep{
{
Expand Down
1 change: 0 additions & 1 deletion pkg/datasources/streams.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"log"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

Expand Down
6 changes: 5 additions & 1 deletion pkg/datasources/system_get_snowflake_platform_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package datasources

import (
"context"
"database/sql"
"errors"
"fmt"
"log"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -35,10 +37,12 @@ func SystemGetSnowflakePlatformInfo() *schema.Resource {
// ReadSystemGetSnowflakePlatformInfo implements schema.ReadFunc.
func ReadSystemGetSnowflakePlatformInfo(d *schema.ResourceData, meta interface{}) error {
db := meta.(*sql.DB)
client := sdk.NewClientFromDB(db)

sel := snowflake.SystemGetSnowflakePlatformInfoQuery()
row := snowflake.QueryRow(db, sel)

acc, err := snowflake.ReadCurrentAccount(db)
acc, err := client.ContextFunctions.Current(context.Background())
if err != nil {
// If not found, mark resource to be removed from state file during apply or refresh
d.SetId("")
Expand Down
Loading
Loading