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

chore: Add ShowByID filtering generation #3227

Merged
merged 19 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f4d01dd
Adding generating ShowByID filtering options from def file
sfc-gh-fbudzynski Nov 25, 2024
a7366ec
Moved show by id filtering to another file
sfc-gh-fbudzynski Nov 26, 2024
bc0fb66
Merge branch 'main' of github.com:Snowflake-Labs/terraform-provider-s…
sfc-gh-fbudzynski Nov 26, 2024
5cb03d1
Remove unused definition in template
sfc-gh-fbudzynski Nov 26, 2024
8f2a301
change showByID Filtering to diference types of filtering based on ne…
sfc-gh-fbudzynski Nov 26, 2024
1599afb
linter adjustemnts
sfc-gh-fbudzynski Nov 26, 2024
ebeffac
Merge branch 'main' of github.com:Snowflake-Labs/terraform-provider-s…
sfc-gh-fbudzynski Nov 28, 2024
dcbae83
add filtering type
sfc-gh-fbudzynski Nov 28, 2024
151741b
first steps into implementing interface
sfc-gh-fbudzynski Nov 28, 2024
93ab8ff
refactor
sfc-gh-fbudzynski Dec 2, 2024
947902d
refactor cleanup
sfc-gh-fbudzynski Dec 2, 2024
4633831
test generator run for streamlits
sfc-gh-fbudzynski Dec 2, 2024
04bc3ce
Merge branch 'main' of github.com:Snowflake-Labs/terraform-provider-s…
sfc-gh-fbudzynski Dec 2, 2024
aaf81f1
rephrase the TODO message
sfc-gh-fbudzynski Dec 2, 2024
6370828
regenerate secrets back to In
sfc-gh-fbudzynski Dec 2, 2024
29b2ad8
enum identifierKind
sfc-gh-fbudzynski Dec 5, 2024
b02da54
showbyid generation with examples
sfc-gh-fbudzynski Dec 5, 2024
9cd65c3
showbyid fix for generating nofiltering
sfc-gh-fbudzynski Dec 5, 2024
9d51aa9
Merge branch 'main' into sdk-generator-show-by-id
sfc-gh-fbudzynski Dec 5, 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
4 changes: 3 additions & 1 deletion pkg/sdk/application_roles_def.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,6 @@ var ApplicationRolesDef = g.NewInterface(
Identifier("ApplicationName", g.KindOfT[AccountObjectIdentifier](), g.IdentifierOptions()).
OptionalLimitFrom().
WithValidation(g.ValidIdentifier, "ApplicationName"),
).ShowByIdOperation()
).ShowByIdOperationWithFiltering(
g.ShowByIDApplicationNameFiltering,
)
3 changes: 2 additions & 1 deletion pkg/sdk/application_roles_impl_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ func (v *applicationRoles) Show(ctx context.Context, request *ShowApplicationRol
}

func (v *applicationRoles) ShowByID(ctx context.Context, id DatabaseObjectIdentifier) (*ApplicationRole, error) {
request := NewShowApplicationRoleRequest().WithApplicationName(id.DatabaseId())
request := NewShowApplicationRoleRequest().
WithApplicationName(id.DatabaseId())
applicationRoles, err := v.Show(ctx, request)
if err != nil {
return nil, err
Expand Down
4 changes: 3 additions & 1 deletion pkg/sdk/connections_def.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,6 @@ var ConnectionDef = g.NewInterface(
Show().
SQL("CONNECTIONS").
OptionalLike(),
).ShowByIdOperation()
).ShowByIdOperationWithFiltering(
g.ShowByIDLikeFiltering,
)
7 changes: 3 additions & 4 deletions pkg/sdk/connections_impl_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,9 @@ func (v *connections) Show(ctx context.Context, request *ShowConnectionRequest)
}

func (v *connections) ShowByID(ctx context.Context, id AccountObjectIdentifier) (*Connection, error) {
connections, err := v.Show(ctx, NewShowConnectionRequest().WithLike(
Like{
Pattern: String(id.Name()),
}))
request := NewShowConnectionRequest().
WithLike(Like{Pattern: String(id.Name())})
connections, err := v.Show(ctx, request)
if err != nil {
return nil, err
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/sdk/poc/generator/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,9 @@ func NewInterface(name string, nameSingular string, identifierKind string, opera
func (i *Interface) NameLowerCased() string {
return startingWithLowerCase(i.Name)
}

// ObjectIdentifierKind returns the level of the object identifier (e.g. for DatabaseObjectIdentifier, it returns the prefix "Database")
func (i *Interface) ObjectIdentifierPrefix() idPrefix {
// return strings.Replace(i.IdentifierKind, "ObjectIdentifier", "", 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove if it doesn't serve any purpose

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 is used to fill the "In" filtering with ObjectIdentifier prefixes based on the object identifier type e.g. DatabaseObjectIdentifier etc. So that the filtering is defined once, and is applicable for any ObejctIdentifier type.

return identifierStringToPrefix(i.IdentifierKind)
}
19 changes: 15 additions & 4 deletions pkg/sdk/poc/generator/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ type Operation struct {
DescribeKind *DescriptionMappingKind
// DescribeMapping is a definition of mapping needed by Operation kind of OperationKindDescribe
DescribeMapping *Mapping
// ShowByIDFiltering defines a kind of filterings performed in ShowByID operation
ShowByIDFiltering []ShowByIDFiltering
}

type Mapping struct {
Expand Down Expand Up @@ -85,11 +87,10 @@ func addDescriptionMapping(op *Operation, from, to *Field) {
op.DescribeMapping = newMapping("convert", from, to)
}

func (i *Interface) newNoSqlOperation(kind string) *Interface {
func newNoSqlOperation(kind string) *Operation {
operation := newOperation(kind, "placeholder").
withOptionsStruct(nil)
i.Operations = append(i.Operations, operation)
return i
return operation
}

func (i *Interface) newSimpleOperation(kind string, doc string, queryStruct *QueryStruct, helperStructs ...IntoField) *Interface {
Expand Down Expand Up @@ -161,7 +162,17 @@ func (i *Interface) ShowOperation(doc string, dbRepresentation *dbStruct, resour
}

func (i *Interface) ShowByIdOperation() *Interface {
return i.newNoSqlOperation(string(OperationKindShowByID))
op := newNoSqlOperation(string(OperationKindShowByID))
i.Operations = append(i.Operations, op)
return i
}

func (i *Interface) ShowByIdOperationWithFiltering(filter ShowByIDFilteringKind, filtering ...ShowByIDFilteringKind) *Interface {
Copy link
Collaborator Author

@sfc-gh-fbudzynski sfc-gh-fbudzynski Dec 5, 2024

Choose a reason for hiding this comment

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

New Operation forcing users to use filtering options (as discussed with @sfc-gh-asawicki)
ShowByIdOperation without filtering should be removed later

op := newNoSqlOperation(string(OperationKindShowByID))
op.ObjectInterface = i
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The operation.ObjectInterface is being set here, because without it operation.ObjectInterface.ObjectIdentifierKind() can't be used before executing the template. It results in nil pointer dereference, same as forInterface.NameLowerCased().

I wanted to use it before the template execution to invoke the filtering just by calling {{ .WithFiltering }} in the template.

I'm open for discussion on this one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm fine with assigning it in the constructor (newNoSqlOperation in this case), it's not bad and imo could be added to other constructors that are Interface methods

op.withFiltering(append([]ShowByIDFilteringKind{filter}, filtering...)...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: you can do it a little bit shorter with (append(filtering, filter)...)

i.Operations = append(i.Operations, op)
return i
}

func (i *Interface) DescribeOperation(describeKind DescriptionMappingKind, doc string, dbRepresentation *dbStruct, resourceRepresentation *plainStruct, queryStruct *QueryStruct) *Interface {
Expand Down
113 changes: 113 additions & 0 deletions pkg/sdk/poc/generator/show_by_id_filtering.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package generator

import (
"fmt"
"log"
)

type ShowByIDFilteringKind uint

const (
ShowByIDLikeFiltering ShowByIDFilteringKind = iota
ShowByIDInFiltering
ShowByIDExtendedInFiltering
sfc-gh-jmichalak marked this conversation as resolved.
Show resolved Hide resolved
ShowByIDApplicationNameFiltering
ShowByIDNoFiltering
)

type idPrefix string

const (
AccountIdentifierPrefix idPrefix = "Account"
DatabaseIdentifierPrefix idPrefix = "Database"
SchemaIdentifierPrefix idPrefix = "Schema"
)

func identifierStringToPrefix(s string) idPrefix {
switch s {
case "AccountObjectIdentifier":
return AccountIdentifierPrefix
case "DatabaseObjectIdentifier":
return DatabaseIdentifierPrefix
case "SchemaObjectIdentifier":
return SchemaIdentifierPrefix
default:
return ""
}
}

type ShowByIDFiltering interface {
WithFiltering() string
}

type showByIDFilter struct {
Name string
Kind string
Args string
}

func (s *showByIDFilter) WithFiltering() string {
return fmt.Sprintf("With%s(%s{%s})", s.Name, s.Kind, s.Args)
}

func newShowByIDFiltering(name, kind, args string) ShowByIDFiltering {
return &showByIDFilter{
Name: name,
Kind: kind,
Args: args,
}
}

func newShowByIDNoFiltering() ShowByIDFiltering {
return newShowByIDFiltering("NoFiltering", "", "")
}

func newShowByIDLikeFiltering() ShowByIDFiltering {
return newShowByIDFiltering("Like", "Like", "Pattern: String(id.Name())")
}

func newShowByIDInFiltering(identifierKind idPrefix) ShowByIDFiltering {
return newShowByIDFiltering("In", "In", fmt.Sprintf("%[1]v: id.%[1]vId()", identifierKind))
}

func newShowByIDExtendedInFiltering(identifierKind idPrefix) ShowByIDFiltering {
return newShowByIDFiltering("In", "ExtendedIn", fmt.Sprintf("In: In{%[1]v: id.%[1]vId()}", identifierKind))
}

type showByIDApplicationFilter struct {
showByIDFilter
}

func (s *showByIDApplicationFilter) WithFiltering() string {
return fmt.Sprintf("With%s(%s)", s.Name, s.Args)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

showByIDApplicationFilter uses different formatting than usual so this is an example of how to leverage the showByIDFilter struct if other formatting is needed

}

func newShowByIDApplicationFiltering() ShowByIDFiltering {
return &showByIDApplicationFilter{
showByIDFilter: showByIDFilter{
Name: "ApplicationName",
Kind: "",
Args: "id.DatabaseId()",
},
}
}

func (s *Operation) withFiltering(filtering ...ShowByIDFilteringKind) *Operation {
for _, filteringKind := range filtering {
switch filteringKind {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Discussed with @sfc-gh-jmichalak to reject the usage of

map[ShowByIDFilteringKind]func(idPrefix) ShowByIDFiltering

and use a switch instead, so that there is no need to append more arguments to each function if new filtering is added

case ShowByIDInFiltering:
sfc-gh-jmichalak marked this conversation as resolved.
Show resolved Hide resolved
s.ShowByIDFiltering = append(s.ShowByIDFiltering, newShowByIDInFiltering(s.ObjectInterface.ObjectIdentifierPrefix()))
case ShowByIDExtendedInFiltering:
s.ShowByIDFiltering = append(s.ShowByIDFiltering, newShowByIDExtendedInFiltering(s.ObjectInterface.ObjectIdentifierPrefix()))
case ShowByIDLikeFiltering:
s.ShowByIDFiltering = append(s.ShowByIDFiltering, newShowByIDLikeFiltering())
case ShowByIDApplicationNameFiltering:
s.ShowByIDFiltering = append(s.ShowByIDFiltering, newShowByIDApplicationFiltering())
case ShowByIDNoFiltering:
s.ShowByIDFiltering = []ShowByIDFiltering{newShowByIDNoFiltering()}
default:
log.Println("No showByID filtering found for kind:", filteringKind)
}
}
return s
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{{- /*gotype: github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/poc/generator.Interface*/ -}}

{{ $impl := .NameLowerCased }}

{{ range .Operations }}
{{ if and (eq .Name "Show") .ShowMapping }}
func (v *{{ $impl }}) Show(ctx context.Context, request *{{ .OptsField.DtoDecl }}) ([]{{ .ShowMapping.To.Name }}, error) {
Expand All @@ -14,8 +15,13 @@
}
{{ else if eq .Name "ShowByID" }}
func (v *{{ $impl }}) ShowByID(ctx context.Context, id {{ .ObjectInterface.IdentifierKind }}) (*{{ .ObjectInterface.NameSingular }}, error) {
// TODO: adjust request if e.g. LIKE is supported for the resource
{{ $impl }}, err := v.Show(ctx, NewShow{{ .ObjectInterface.NameSingular }}Request())
request := NewShow{{ .ObjectInterface.NameSingular }}Request()
{{- range .ShowByIDFiltering }}
{{- if not (eq .Name "NoFiltering") -}}.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

mind the dot at the end here
checking for "NoFiltering" name allows for proper adding filtering options, one after another, and if NoFiltering option is applied, the dot will not be present

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be easier if the .ShowByIDFiltering would have no items? Then the check wouldn't be necessary, plus you can have two items in the collection, one being NoFiltering and the second SomeFilter. What I mean is, there's nothing you can do now to prevent unwanted behavior. Maybe instead of NoFiltering filtering option, we should rename the default function to emphasize the fact that no filtering is generated there (enum value is also good, but as mentioned, it could be used with other filters making it just a tag value).

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1, I think having empty slice and removing this if would be better.

{{ .WithFiltering }}
{{- end }}
{{- end }}
{{ $impl }}, err := v.Show(ctx, request)
if err != nil {
return nil, err
}
Expand Down
26 changes: 14 additions & 12 deletions pkg/sdk/secrets_def.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,15 +241,17 @@ var SecretsDef = g.NewInterface(
SQL("SECRETS").
OptionalLike().
OptionalExtendedIn(),
).ShowByIdOperation().
DescribeOperation(
g.DescriptionMappingKindSingleValue,
"https://docs.snowflake.com/en/sql-reference/sql/desc-secret",
secretDetailsDbRow,
secretDetails,
g.NewQueryStruct("DescribeSecret").
Describe().
SQL("SECRET").
Name().
WithValidation(g.ValidIdentifier, "name"),
)
).ShowByIdOperationWithFiltering(
g.ShowByIDLikeFiltering,
g.ShowByIDExtendedInFiltering,
).DescribeOperation(
g.DescriptionMappingKindSingleValue,
"https://docs.snowflake.com/en/sql-reference/sql/desc-secret",
secretDetailsDbRow,
secretDetails,
g.NewQueryStruct("DescribeSecret").
Describe().
SQL("SECRET").
Name().
WithValidation(g.ValidIdentifier, "name"),
)
2 changes: 0 additions & 2 deletions pkg/sdk/secrets_dto_builders_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions pkg/sdk/secrets_impl_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ func (v *secrets) Show(ctx context.Context, request *ShowSecretRequest) ([]Secre
}

func (v *secrets) ShowByID(ctx context.Context, id SchemaObjectIdentifier) (*Secret, error) {
request := NewShowSecretRequest().WithIn(ExtendedIn{In: In{Schema: id.SchemaId()}}).WithLike(Like{String(id.Name())})
request := NewShowSecretRequest().
WithLike(Like{Pattern: String(id.Name())}).
WithIn(ExtendedIn{In: In{Schema: id.SchemaId()}})
secrets, err := v.Show(ctx, request)
if err != nil {
return nil, err
Expand All @@ -78,8 +80,7 @@ func (r *CreateWithOAuthClientCredentialsFlowSecretRequest) toOpts() *CreateWith
IfNotExists: r.IfNotExists,
name: r.name,
ApiIntegration: r.ApiIntegration,

Comment: r.Comment,
Comment: r.Comment,
}

if r.OauthScopes != nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/sdk/session_policies_def.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ var SessionPoliciesDef = g.NewInterface(
Show().
SQL("SESSION POLICIES"),
).
ShowByIdOperationWithFiltering(
g.ShowByIDNoFiltering,
).
DescribeOperation(
g.DescriptionMappingKindSingleValue,
"https://docs.snowflake.com/en/sql-reference/sql/desc-session-policy",
Expand Down
4 changes: 2 additions & 2 deletions pkg/sdk/session_policies_impl_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ func (v *sessionPolicies) Show(ctx context.Context, request *ShowSessionPolicyRe
}

func (v *sessionPolicies) ShowByID(ctx context.Context, id SchemaObjectIdentifier) (*SessionPolicy, error) {
sessionPolicies, err := v.Show(ctx, NewShowSessionPolicyRequest())
request := NewShowSessionPolicyRequest()
sessionPolicies, err := v.Show(ctx, request)
if err != nil {
return nil, err
}

return collections.FindFirst(sessionPolicies, func(r SessionPolicy) bool { return r.Name == id.Name() })
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/sdk/streamlits_def.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ var StreamlitsDef = g.NewInterface(
OptionalLike().
OptionalIn().
OptionalLimit(),
).ShowByIdOperation().DescribeOperation(
).ShowByIdOperationWithFiltering(
g.ShowByIDLikeFiltering,
g.ShowByIDInFiltering,
).DescribeOperation(
g.DescriptionMappingKindSingleValue,
"https://docs.snowflake.com/en/sql-reference/sql/desc-streamlit",
g.DbStruct("streamlitsDetailRow").
Expand Down
9 changes: 5 additions & 4 deletions pkg/sdk/streamlits_impl_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ func (v *streamlits) Show(ctx context.Context, request *ShowStreamlitRequest) ([
}

func (v *streamlits) ShowByID(ctx context.Context, id SchemaObjectIdentifier) (*Streamlit, error) {
request := NewShowStreamlitRequest().WithIn(In{Schema: id.SchemaId()}).WithLike(Like{String(id.Name())})
request := NewShowStreamlitRequest().
WithLike(Like{Pattern: String(id.Name())}).
WithIn(In{Schema: id.SchemaId()})
streamlits, err := v.Show(ctx, request)
if err != nil {
return nil, err
Expand Down Expand Up @@ -92,9 +94,8 @@ func (r *AlterStreamlitRequest) toOpts() *AlterStreamlitOptions {
RootLocation: r.Set.RootLocation,
MainFile: r.Set.MainFile,
QueryWarehouse: r.Set.QueryWarehouse,

Comment: r.Set.Comment,
Title: r.Set.Title,
Comment: r.Set.Comment,
Title: r.Set.Title,
}

if r.Set.ExternalAccessIntegrations != nil {
Expand Down
Loading