chore: Add ShowByID filtering generation #3227

sfc-gh-fbudzynski · 2024-11-26T15:06:38Z

Changes

Added ShowByID filtering options to implement_functions template
New filed in Operation struct with Filtering options

To generate filtering options, add filtering options in ShowByIDOperation:

).ShowByIdOperation(
	g.ShowByIDLikeFiltering,
	g.ShowByIDExtendedInFiltering,
)

Test Plan

Generation for Like and ExtendedIn for SchemaObjectidentifier (Secrets):

acceptance tests
integration tests
unit tests

Generation for Like and In for SchemaObjectIdentifier (Streamlits):

acceptance tests
integration tests
unit tests

Generation for Like for AccountObjectIdentifier (Connections):

acceptance tests
integration tests
unit tests

Generation for ApplicationName for DatabaseObjectIdentifier (Application Roles):

acceptance tests
integration tests
unit tests

Notes

WithIn and WithLike are mostly used in ShowByID()
no objects use Limit and StartsWith in ShowByID()
application_roles is the only DatabaseObejctIdentifier with a _def file
application_roles does not implement any of the common filtering options e.g. OptionalLike() or OptionalIn()
application_roles implement WithApplicationName filtering option that has been added

TODO

change ShowByIdOperation() to ShowByIdOperationWithoutFiltering() and remove NoFiltering Option

…nowflake into sdk-generator-show-by-id

…eded Identifier value

github-actions · 2024-11-26T17:00:48Z

Integration tests failure for 1599afb36a36122395af19213ab1a4562b220e90

pkg/datasources/secrets.go

pkg/sdk/poc/generator/templates/sub_templates/implementation_functions.tmpl

pkg/sdk/poc/generator/show_by_id_filtering.go

sfc-gh-jmichalak · 2024-11-27T10:58:49Z

pkg/sdk/poc/generator/show_by_id_filtering.go

+type ShowByIDFiltering struct {
+	Kind            string
+	Args            string
+	IdentifierBased bool


I wonder if we could use an enum instead? What if we have more showByIDs which have a different behavior? Such flag could be confusing.

You're right, that would probably be a better approach than just a flag

pkg/sdk/poc/generator/show_by_id_filtering.go

…nowflake into sdk-generator-show-by-id

sfc-gh-fbudzynski · 2024-12-02T13:04:25Z

pkg/sdk/poc/generator/operation.go

-	return i.newNoSqlOperation(string(OperationKindShowByID))
+func (i *Interface) ShowByIdOperation(filtering ...ShowByIDFilteringKind) *Interface {
+	op := i.newNoSqlOperation(string(OperationKindShowByID))
+	op.ObjectInterface = i


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.

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

github-actions · 2024-12-02T14:47:27Z

Integration tests failure for 46338317e73c3293098f0730143ed6f0e30a7d65

github-actions · 2024-12-02T14:52:43Z

Integration tests failure for 04bc3ce16b052218b1b3ad833e6b4ca843c251a1

github-actions · 2024-12-02T14:53:04Z

Integration tests failure for aaf81f1f9b74900a639cf369a122edf240e15904

github-actions · 2024-12-02T15:13:27Z

Integration tests failure for 637082805361888801b0c3d59ad46a85cbbe634e

sfc-gh-jmichalak · 2024-12-03T09:01:07Z

pkg/sdk/poc/generator/show_by_id_filtering.go

+	return newShowByIDFiltering("In", "In", "%[1]v: id.%[1]vId()", &identifierKind)
+}
+
+func newShowByIDExtendedInFiltering(identifierKind string) ShowByIDFiltering {


Can we have an enum for identifierKind?

sfc-gh-jmichalak · 2024-12-03T09:12:57Z

pkg/sdk/poc/generator/show_by_id_filtering.go

+	ShowByIDLimitFiltering:      newShowByIDLimitFiltering,
+}
+
+func newShowByIDFiltering(name, kind, args string, identifierKind *string) ShowByIDFiltering {


How about using a variadic function?

will be done by formatting arguments in the create function (as discussed verbally). This way the identifier kind is removed from newShowByIDFiltering() function and there is no need for variadic anymore.

pkg/sdk/secrets_def.go

pkg/sdk/streamlits_def.go

pkg/sdk/poc/generator/templates/sub_templates/implementation_functions.tmpl

pkg/sdk/poc/generator/operation.go

sfc-gh-fbudzynski · 2024-12-05T15:10:07Z

pkg/sdk/poc/generator/show_by_id_filtering.go

+
+func (s *Operation) withFiltering(filtering ...ShowByIDFilteringKind) *Operation {
+	for _, filteringKind := range filtering {
+		switch filteringKind {


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

sfc-gh-fbudzynski · 2024-12-05T15:12:26Z

pkg/sdk/poc/generator/show_by_id_filtering.go

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


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

sfc-gh-fbudzynski · 2024-12-05T15:16:56Z

pkg/sdk/poc/generator/templates/sub_templates/implementation_functions.tmpl

-        {{ $impl }}, err := v.Show(ctx, NewShow{{ .ObjectInterface.NameSingular }}Request())
+            request := NewShow{{ .ObjectInterface.NameSingular }}Request()
+            {{- range .ShowByIDFiltering }}
+                {{- if not (eq .Name "NoFiltering") -}}.


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

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).

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

sfc-gh-fbudzynski · 2024-12-05T15:19:08Z

pkg/sdk/poc/generator/operation.go

+	return i
+}
+
+func (i *Interface) ShowByIdOperationWithFiltering(filter ShowByIDFilteringKind, filtering ...ShowByIDFilteringKind) *Interface {


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

github-actions · 2024-12-05T17:13:11Z

Integration tests failure for 9cd65c3b480da5f80e0fae4fa39cfd921aeff55d

github-actions · 2024-12-05T18:01:51Z

Integration tests failure for 9d51aa9cb8cff056971ce5b4d6af669ec9cb76e7

sfc-gh-jcieslak · 2024-12-09T10:29:50Z

pkg/sdk/poc/generator/interface.go

-	return strings.Replace(i.IdentifierKind, "ObjectIdentifier", "", 1)
+// 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)


remove if it doesn't serve any purpose

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.

sfc-gh-jcieslak · 2024-12-09T10:34:54Z

pkg/sdk/poc/generator/operation.go

 	op.ObjectInterface = i
-	op.withFiltering(filtering...)
+	op.withFiltering(append([]ShowByIDFilteringKind{filter}, filtering...)...)


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

sfc-gh-jcieslak · 2024-12-09T10:43:53Z

pkg/sdk/poc/generator/templates/sub_templates/implementation_functions.tmpl

-        {{ $impl }}, err := v.Show(ctx, NewShow{{ .ObjectInterface.NameSingular }}Request())
+            request := NewShow{{ .ObjectInterface.NameSingular }}Request()
+            {{- range .ShowByIDFiltering }}
+                {{- if not (eq .Name "NoFiltering") -}}.


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).

🤖 I have created a release *beep* *boop* --- ## [0.100.0](v0.99.0...v0.100.0) (2024-12-12) ### 🎉 **What's new:** * Account v1 readiness ([#3236](#3236)) ([5df33a8](5df33a8)) * Account v1 readiness generated files ([#3242](#3242)) ([3df59dd](3df59dd)) * Account v1 readiness resource ([#3252](#3252)) ([8f5698d](8f5698d)) * Add a new account roles data source ([#3257](#3257)) ([b3d6b9e](b3d6b9e)) * Add account data source ([#3261](#3261)) ([6087fc9](6087fc9)) * Add all other functions and procedures implementations ([#3275](#3275)) ([7a6f68d](7a6f68d)) * Basic functions implementation ([#3269](#3269)) ([6d4a103](6d4a103)) * Basic procedures implementation ([#3271](#3271)) ([933335f](933335f)) * Docs, test, and missing parameter ([#3280](#3280)) ([10517f3](10517f3)) * Functions and procedures schemas and generated sources ([#3262](#3262)) ([9b70f87](9b70f87)) * Functions sdk update ([#3254](#3254)) ([fc1eace](fc1eace)) * Handle missing fields in function and procedure ([#3273](#3273)) ([53e7a0a](53e7a0a)) * Procedures schemas and generated sources ([#3263](#3263)) ([211ad46](211ad46)) * Procedures sdk update ([#3255](#3255)) ([682606a](682606a)) * Rework account parameter resource ([#3264](#3264)) ([15aa9c2](15aa9c2)) * Rework data types ([#3244](#3244)) ([05ada91](05ada91)) * support table data type ([#3274](#3274)) ([13401d5](13401d5)) * Tag association v1 readiness ([#3210](#3210)) ([04f6d54](04f6d54)) * Test imports and small fixes ([#3276](#3276)) ([a712195](a712195)) * Unsafe execute v1 readiness ([#3266](#3266)) ([c4f1e8f](c4f1e8f)) * Use new data types in sql builder for functions and procedures ([#3247](#3247)) ([69f677a](69f677a)) ### 🔧 **Misc** * Add ShowByID filtering generation ([#3227](#3227)) ([548ec42](548ec42)) * Adress small task-related todos ([#3243](#3243)) ([40de9ae](40de9ae)) * Apply masking ([#3234](#3234)) ([c209a8a](c209a8a)) * fix missing references in toOpts and changes with newlines ([#3240](#3240)) ([246547f](246547f)) * function tests ([#3279](#3279)) ([5af6efb](5af6efb)) * Improve config builders ([#3207](#3207)) ([425787c](425787c)) * Revert to proper env ([#3238](#3238)) ([5d4ed3b](5d4ed3b)) * Use service user for ci ([#3228](#3228)) ([2fb50d7](2fb50d7)) ### 🐛 **Bug fixes:** * Make blocked_roles_field optional in OAuth security integrations ([#3267](#3267)) ([7197b57](7197b57)) * Minor fixes ([#3231](#3231)) ([1863bf6](1863bf6)) * Minor fixes 2 ([#3230](#3230)) ([73b7e74](73b7e74)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>

## Changes - removed generation of integration_tests file - added a print for the user to create the integration tests manually - changed the `ShowByIdOperation` name to `ShowByIdOperationNoFiltering` for no automatic generation of filtering options - adjusted the template for ShowByID filtering - removed `ShowByIDNoFiltering` option (replaced by `ShowByIdOperationNoFiltering`) - adjusted README of the SDK generator to the current state ## References  * #3227 * #3240

sfc-gh-fbudzynski added 6 commits November 25, 2024 16:55

Adding generating ShowByID filtering options from def file

f4d01dd

Moved show by id filtering to another file

a7366ec

Merge branch 'main' of github.com:Snowflake-Labs/terraform-provider-s…

bc0fb66

…nowflake into sdk-generator-show-by-id

Remove unused definition in template

5cb03d1

change showByID Filtering to diference types of filtering based on ne…

8f2a301

…eded Identifier value

linter adjustemnts

1599afb

sfc-gh-fbudzynski changed the title ~~Chore: Add ShowByID filtering generation~~ chore: Add ShowByID filtering generation Nov 26, 2024

sfc-gh-fbudzynski requested review from sfc-gh-jmichalak, sfc-gh-jcieslak and sfc-gh-asawicki November 27, 2024 10:42

sfc-gh-fbudzynski marked this pull request as ready for review November 27, 2024 10:43

sfc-gh-jmichalak requested changes Nov 27, 2024

View reviewed changes

sfc-gh-jmichalak reviewed Nov 27, 2024

View reviewed changes

pkg/sdk/poc/generator/show_by_id_filtering.go Show resolved Hide resolved

sfc-gh-fbudzynski added 9 commits November 28, 2024 09:56

Merge branch 'main' of github.com:Snowflake-Labs/terraform-provider-s…

ebeffac

…nowflake into sdk-generator-show-by-id

add filtering type

dcbae83

first steps into implementing interface

151741b

refactor

93ab8ff

refactor cleanup

947902d

test generator run for streamlits

4633831

Merge branch 'main' of github.com:Snowflake-Labs/terraform-provider-s…

04bc3ce

…nowflake into sdk-generator-show-by-id

rephrase the TODO message

aaf81f1

regenerate secrets back to In

6370828

sfc-gh-fbudzynski commented Dec 2, 2024

View reviewed changes

sfc-gh-fbudzynski requested a review from sfc-gh-jmichalak December 2, 2024 15:32

sfc-gh-jmichalak reviewed Dec 3, 2024

View reviewed changes

sfc-gh-asawicki requested a review from sfc-gh-jmichalak December 3, 2024 15:08

sfc-gh-asawicki requested changes Dec 3, 2024

View reviewed changes

sfc-gh-fbudzynski added 3 commits December 5, 2024 09:15

enum identifierKind

29b2ad8

showbyid generation with examples

b02da54

showbyid fix for generating nofiltering

9cd65c3

sfc-gh-fbudzynski commented Dec 5, 2024

View reviewed changes

Merge branch 'main' into sdk-generator-show-by-id

9d51aa9

sfc-gh-fbudzynski requested a review from sfc-gh-asawicki December 5, 2024 15:36

sfc-gh-asawicki approved these changes Dec 5, 2024

View reviewed changes

sfc-gh-jcieslak reviewed Dec 9, 2024

View reviewed changes

sfc-gh-jmichalak approved these changes Dec 9, 2024

View reviewed changes

sfc-gh-fbudzynski merged commit 548ec42 into main Dec 9, 2024
8 of 9 checks passed

sfc-gh-fbudzynski deleted the sdk-generator-show-by-id branch December 9, 2024 13:15

snowflake-release-please bot mentioned this pull request Dec 9, 2024

chore(main): release 0.100.0 #3237

Merged

sfc-gh-fbudzynski mentioned this pull request Dec 9, 2024

chore: remove generating integration tests placeholder file #3256

Merged

sfc-gh-jmichalak mentioned this pull request Dec 10, 2024

chore: Make ShowByID configurable in generator #2822

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Add ShowByID filtering generation #3227

chore: Add ShowByID filtering generation #3227

sfc-gh-fbudzynski commented Nov 26, 2024 •

edited

Loading

github-actions bot commented Nov 26, 2024

sfc-gh-jmichalak Nov 27, 2024

sfc-gh-fbudzynski Nov 28, 2024

sfc-gh-fbudzynski Dec 2, 2024

sfc-gh-jcieslak Dec 3, 2024

github-actions bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

sfc-gh-jmichalak Dec 3, 2024

sfc-gh-jmichalak Dec 3, 2024

sfc-gh-fbudzynski Dec 5, 2024

sfc-gh-fbudzynski Dec 5, 2024

sfc-gh-fbudzynski Dec 5, 2024

sfc-gh-fbudzynski Dec 5, 2024

sfc-gh-jcieslak Dec 9, 2024

sfc-gh-jmichalak Dec 9, 2024

sfc-gh-fbudzynski Dec 5, 2024 •

edited

Loading

github-actions bot commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

sfc-gh-jcieslak Dec 9, 2024

sfc-gh-fbudzynski Dec 9, 2024

sfc-gh-jcieslak Dec 9, 2024

sfc-gh-jcieslak Dec 9, 2024

chore: Add ShowByID filtering generation #3227

chore: Add ShowByID filtering generation #3227

Conversation

sfc-gh-fbudzynski commented Nov 26, 2024 • edited Loading

Changes

Test Plan

Notes

TODO

github-actions bot commented Nov 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-fbudzynski Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-fbudzynski commented Nov 26, 2024 •

edited

Loading

sfc-gh-fbudzynski Dec 5, 2024 •

edited

Loading