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

feat: use sdk for procedures #2450

Merged
merged 12 commits into from
Feb 13, 2024
20 changes: 10 additions & 10 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Migration guide

This document is meant to help you migrate your Terraform config to the new newest version. In migration guides, we will only
This document is meant to help you migrate your Terraform config to the new newest version. In migration guides, we will only
describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behavior
across different versions.

Expand All @@ -14,13 +14,13 @@ It is noted as a behavior change but in some way it is not; with the previous im

We will consider adding `NOT NULL` back because it can be set by `ALTER COLUMN columnX SET NOT NULL`, but first we want to revisit the whole resource design.

## vX.XX.X -> v0.85.0
### snowflake_procedure resource changes
#### *(deprecation)* return_behavior
`return_behavior` parameter is deprecated because it is also deprecated in the Snowflake API.

### Migration from old (grant) resources to new ones
Copy link
Collaborator

Choose a reason for hiding this comment

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

why this is removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it looked like it was re-written? at the top is a new migration guide intro.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It was added to point the users to the resource migration guide, I would leave it as it was

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, please revert this whole paragraph. Also, TestInt_CreateProcedures is failing, so we cannot merge it yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

re-added paragraph. Commented out problem in TestInt_CreateProcedures, its an issue I already filed with docs-discuss channel. Included jira ticket in the comment

Copy link
Collaborator Author

@sfc-gh-swinkler sfc-gh-swinkler Feb 12, 2024

Choose a reason for hiding this comment

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

test still failing, but passes locally:

--- PASS: TestInt_CreateProcedures (21.14s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_Java:_returns_result_data_type (2.72s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_Java:_returns_table (2.44s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_Javascript (0.30s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_Javascript:_no_arguments (0.24s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_Scala:_returns_result_data_type (4.02s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_Scala:_returns_table (3.95s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_Python:_returns_result_data_type (4.89s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_Python:_returns_table (1.81s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_SQL:_returns_result_data_type (0.42s)
    --- PASS: TestInt_CreateProcedures/create_procedure_for_SQL:_returns_table (0.35s)
PASS

unsure what is going on, going to re-run tests


In recent changes, we introduced a new grant resources to replace the old ones.
To aid with the migration, we wrote a guide to show one of the possible ways to migrate deprecated resources to their new counter-parts.
As the guide is more general and applies to every version (and provider), we moved it [here](./docs/technical-documentation/resource_migration.md).
### snowflake_function resource changes
#### *(behavior change)* return_type
`return_type` has become force new because there is no way to alter it without dropping and recreating the function.

## v0.84.0 ➞ v0.85.0

Expand All @@ -47,7 +47,7 @@ Force new was added for the following attributes (because no usable SQL alter st
## v0.73.0 ➞ v0.74.0
### Provider configuration changes

In this change we have done a provider refactor to make it more complete and customizable by supporting more options that
In this change we have done a provider refactor to make it more complete and customizable by supporting more options that
were already available in Golang Snowflake driver. This lead to several attributes being added and a few deprecated.
We will focus on the deprecated ones and show you how to adapt your current configuration to the new changes.

Expand All @@ -57,7 +57,7 @@ We will focus on the deprecated ones and show you how to adapt your current conf
provider "snowflake" {
# before
username = "username"

# after
user = "username"
}
Expand Down Expand Up @@ -123,7 +123,7 @@ provider "snowflake" {
provider "snowflake" {
# before
session_params = {}

# after
params = {}
}
Expand Down
3 changes: 2 additions & 1 deletion docs/resources/procedure.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ EOT
- `language` (String) Specifies the language of the stored procedure code.
- `null_input_behavior` (String) Specifies the behavior of the procedure when called with null inputs.
- `packages` (List of String) List of package imports to use for Java / Python procedures. For Java, package imports should be of the form: package_name:version_number, where package_name is snowflake_domain:package. For Python use it should be: ('numpy','pandas','xgboost==1.5.0').
- `return_behavior` (String) Specifies the behavior of the function when returning results
- `return_behavior` (String, Deprecated) Specifies the behavior of the function when returning results
sfc-gh-asawicki marked this conversation as resolved.
Show resolved Hide resolved
- `runtime_version` (String) Required for Python procedures. Specifies Python runtime version.
- `secure` (Boolean) Specifies that the procedure is secure. For more information about secure procedures, see Protecting Sensitive Information with Secure UDFs and Stored Procedures.
sfc-gh-asawicki marked this conversation as resolved.
Show resolved Hide resolved

### Read-Only

Expand Down
67 changes: 38 additions & 29 deletions pkg/datasources/procedures.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package datasources

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

"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/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

Expand Down Expand Up @@ -64,50 +64,59 @@ var proceduresSchema = map[string]*schema.Schema{

func Procedures() *schema.Resource {
return &schema.Resource{
Read: ReadProcedures,
Schema: proceduresSchema,
ReadContext: ReadContextProcedures,
Schema: proceduresSchema,
}
}

func ReadProcedures(d *schema.ResourceData, meta interface{}) error {
func ReadContextProcedures(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
db := meta.(*sql.DB)
client := sdk.NewClientFromDB(db)
databaseName := d.Get("database").(string)
schemaName := d.Get("schema").(string)

currentProcedures, err := snowflake.ListProcedures(databaseName, schemaName, 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] procedures in schema (%s) not found", d.Id())
d.SetId("")
return nil
} else if err != nil {
log.Printf("[DEBUG] unable to parse procedures in schema (%s)", d.Id())
d.SetId("")
return nil
req := sdk.NewShowProcedureRequest()
if databaseName != "" {
sfc-gh-jcieslak marked this conversation as resolved.
Show resolved Hide resolved
req.WithIn(&sdk.In{Database: sdk.NewAccountObjectIdentifier(databaseName)})
}
if schemaName != "" {
req.WithIn(&sdk.In{Schema: sdk.NewDatabaseObjectIdentifier(databaseName, schemaName)})
}
procedures, err := client.Procedures.Show(ctx, req)
if err != nil {
id := fmt.Sprintf(`%v|%v`, databaseName, schemaName)

procedures := []map[string]interface{}{}
d.SetId("")
return diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("Unable to parse procedures in schema (%s)", id),
Detail: "See our document on design decisions for procedures: <LINK (coming soon)>",
},
}
}
proceduresList := []map[string]interface{}{}

for _, procedure := range currentProcedures {
for _, procedure := range procedures {
procedureMap := map[string]interface{}{}

procedureSignatureMap, err := parseArguments(procedure.Arguments.String)
procedureMap["name"] = procedure.Name
procedureMap["database"] = procedure.CatalogName
procedureMap["schema"] = procedure.SchemaName
procedureMap["comment"] = procedure.Description
procedureSignatureMap, err := parseArguments(procedure.Arguments)
if err != nil {
return err
return diag.FromErr(err)
}

procedureMap["name"] = procedure.Name.String
procedureMap["database"] = procedure.DatabaseName.String
procedureMap["schema"] = procedure.SchemaName.String
procedureMap["comment"] = procedure.Comment.String
procedureMap["argument_types"] = procedureSignatureMap["argumentTypes"].([]string)
procedureMap["return_type"] = procedureSignatureMap["returnType"].(string)

procedures = append(procedures, procedureMap)
proceduresList = append(proceduresList, procedureMap)
}

d.SetId(fmt.Sprintf(`%v|%v`, databaseName, schemaName))
return d.Set("procedures", procedures)
if err := d.Set("procedures", proceduresList); err != nil {
return diag.FromErr(err)
}
return nil
}

func parseArguments(arguments string) (map[string]interface{}, error) {
Expand Down
89 changes: 27 additions & 62 deletions pkg/datasources/procedures_acceptance_test.go
Original file line number Diff line number Diff line change
@@ -1,83 +1,48 @@
package datasources_test

import (
"fmt"
"strings"
"testing"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
"github.com/hashicorp/terraform-plugin-testing/config"
"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_Procedures(t *testing.T) {
databaseName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
schemaName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
procedureName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
procedureWithArgumentsName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
resource.ParallelTest(t, resource.TestCase{
Providers: providers(),
procNameOne := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
procNameTwo := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
dataSourceName := "data.snowflake_procedures.procedures"
m := func() map[string]config.Variable {
return map[string]config.Variable{
"database": config.StringVariable(acc.TestDatabaseName),
"schema": config.StringVariable(acc.TestSchemaName),
"proc_name_one": config.StringVariable(procNameOne),
"proc_name_two": config.StringVariable(procNameTwo),
}
}
configVariables := m()
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{
{
Config: procedures(databaseName, schemaName, procedureName, procedureWithArgumentsName),
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Procedures/complete"),
ConfigVariables: configVariables,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.snowflake_procedures.t", "database", databaseName),
resource.TestCheckResourceAttr("data.snowflake_procedures.t", "schema", schemaName),
resource.TestCheckResourceAttrSet("data.snowflake_procedures.t", "procedures.#"),
// resource.TestCheckResourceAttr("data.snowflake_procedures.t", "procedures.#", "3"),
resource.TestCheckResourceAttr(dataSourceName, "database", acc.TestDatabaseName),
resource.TestCheckResourceAttr(dataSourceName, "schema", acc.TestSchemaName),
resource.TestCheckResourceAttrSet(dataSourceName, "procedures.#"),
// resource.TestCheckResourceAttr(dataSourceName, "procedures.#", "3"),
sfc-gh-jcieslak marked this conversation as resolved.
Show resolved Hide resolved
// Extra 1 in procedure count above due to ASSOCIATE_SEMANTIC_CATEGORY_TAGS appearing in all "SHOW PROCEDURES IN ..." commands
),
},
},
})
}

func procedures(databaseName string, schemaName string, procedureName string, procedureWithArgumentsName string) string {
s := `
resource "snowflake_database" "test_database" {
name = "%v"
comment = "Terraform acceptance test"
}

resource "snowflake_schema" "test_schema" {
name = "%v"
database = snowflake_database.test_database.name
comment = "Terraform acceptance test"
}

resource "snowflake_procedure" "test_proc_simple" {
name = "%v"
database = snowflake_database.test_database.name
schema = snowflake_schema.test_schema.name
return_type = "VARCHAR"
language = "JAVASCRIPT"
statement = <<-EOF
return "Hi"
EOF
}

resource "snowflake_procedure" "test_proc" {
name = "%v"
database = snowflake_database.test_database.name
schema = snowflake_schema.test_schema.name
arguments {
name = "arg1"
type = "varchar"
}
comment = "Terraform acceptance test"
return_type = "varchar"
language = "JAVASCRIPT"
statement = <<-EOF
var X=1
return X
EOF
}

data snowflake_procedures "t" {
database = snowflake_database.test_database.name
schema = snowflake_schema.test_schema.name
depends_on = [snowflake_procedure.test_proc_simple, snowflake_procedure.test_proc]
}
`
return fmt.Sprintf(s, databaseName, schemaName, procedureName, procedureWithArgumentsName)
}
49 changes: 49 additions & 0 deletions pkg/datasources/testdata/TestAcc_Procedures/complete/test.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
variable "proc_name_one" {
type = string
}

variable "proc_name_two" {
type = string
}

variable "database" {
type = string
}

variable "schema" {
type = string
}

resource "snowflake_procedure" "test_proc_one" {
name = var.proc_name_one
database = var.database
schema = var.schema
return_type = "VARCHAR"
language = "JAVASCRIPT"
statement = <<-EOF
return "Hi"
EOF
}

resource "snowflake_procedure" "test_proc_two" {
name = var.proc_name_two
database = var.database
schema = var.schema
arguments {
name = "arg1"
type = "varchar"
}
comment = "Terraform acceptance test"
return_type = "varchar"
language = "JAVASCRIPT"
statement = <<-EOF
var X=1
return X
EOF
}

data "snowflake_procedures" "procedures" {
database = var.database
schema = var.schema
depends_on = [snowflake_procedure.test_proc_one, snowflake_procedure.test_proc_two]
}
Loading
Loading