feat: Tasks v1 readiness - SDK part #3086

sfc-gh-jcieslak · 2024-09-18T08:19:24Z

Changes

Generated assertions for task
Added a few missing fields in Create, Alter, and outputs
Added CreateOrAlter support
Adjusted unit and integration tests
Adjusted validation generator a bit (It was panic-ing before for that case; it was never used before)
Adjusted resource and datasource to use new SDK

References

CREATE TASK

github-actions · 2024-09-18T08:39:00Z

Integration tests failure for 1224a7d7c203327688f6cc07481276983f8b9945

github-actions · 2024-09-18T10:43:19Z

Integration tests failure for 56b338b6a902ea1743349a13e1bda715031fa9ef

github-actions · 2024-09-18T10:59:37Z

Integration tests failure for e076055b3e1213e5518abbf3e6a4f14d22f2d153

github-actions · 2024-09-18T13:01:02Z

Integration tests failure for 5e808d1a232ee1c2cae7acf735092af6a20f01e1

pkg/acceptance/bettertestspoc/assert/objectassert/task_snowflake_ext.go

pkg/sdk/tasks_validations_gen.go

pkg/sdk/tasks_gen_test.go

pkg/sdk/tasks_def.go

pkg/sdk/testint/tasks_gen_integration_test.go

github-actions · 2024-09-20T15:40:13Z

Integration tests failure for 554fc90c3931e8ac77d5121095616a83633d339e

github-actions · 2024-09-23T09:30:07Z

Integration tests failure for 87638ea7c7be33c93a211ea5a26e43d5b752bbc9

github-actions · 2024-09-23T13:59:24Z

Integration tests failure for fc5586ec5c63d5bae081066e85bf8161387b169b

github-actions · 2024-09-23T14:00:56Z

Integration tests failure for a783bec3eea98546233ae6dee44e012cc08b9020

sfc-gh-asawicki · 2024-10-01T19:12:49Z

pkg/acceptance/bettertestspoc/assert/objectassert/task_snowflake_ext.go

+	return t
+}
+
+func (t *TaskAssert) HasPredecessors(ids ...sdk.SchemaObjectIdentifier) *TaskAssert {


nit pick: in such assertions (when we require these two lists to match exactly without taking order into account - but this does not matter) it is useful to not only list the missing ids but also list the unexpected ones (and this is how it is usually implemented in the assertion libraries)

Implemented in: Changed the assertions, and added InAnyOrder suffix to this function (on failure all items are printed, so It will be known what was an unexpected item).

sfc-gh-asawicki · 2024-10-01T19:22:39Z

pkg/resources/task.go

@@ -182,7 +182,7 @@ func ReadTask(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}

-	if err := d.Set("enabled", task.IsStarted()); err != nil {
+	if err := d.Set("enabled", task.State == sdk.TaskStateStarted); err != nil {


nit: why we get rid of the helper func from SDK?

Brought back in

sfc-gh-asawicki · 2024-10-01T19:28:52Z

pkg/resources/task.go

@@ -182,7 +182,7 @@ func ReadTask(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}

-	if err := d.Set("enabled", task.IsStarted()); err != nil {
+	if err := d.Set("enabled", task.State == sdk.TaskStateStarted); err != nil {


do we want to set enabled if this is not set in the config? (maybe it is already a topic for the next change but had to ask; we are going away from the defaults for such attributes; it's also similar in a way to the warehouse dilemma we had for the state that can be changes externally - let's at least discuss it)

Don't remember where, but I wrote somewhere that it's very hard or impossible to act on the default enabled value (not set in the config). That's why I made it required, so certain operations are easier because we know what state are we aiming with. This was truly painful during the update where suspend/resume logic is messy because of the Snowflake limitations. Maybe it will be possible to turn it into a tri-valued bool, but for now, for simplicity's sake, I would stay with the required regular bool.

🤖 I have created a release *beep* *boop* --- ## [0.97.0](v0.96.0...v0.97.0) (2024-10-10) ### 🎉 **What's new:** * Add secret to sdk ([#3091](#3091)) ([7430aee](7430aee)) * Add service user and legacy service user resources ([#3119](#3119)) ([0e88e08](0e88e08)) * Handle all Task parameters in SDK ([#3103](#3103)) ([08ae072](08ae072)) * Stream on external table resource ([#3122](#3122)) ([d837341](d837341)) * Stream on table resource ([#3109](#3109)) ([97fa9b4](97fa9b4)) * Tasks v1 readiness - SDK part ([#3086](#3086)) ([0a77383](0a77383)) * Upgrade stream sdk ([#3105](#3105)) ([ad5fa11](ad5fa11)) ### 🔧 **Misc** * Add pre check to each datasource ([#3065](#3065)) ([560ab6b](560ab6b)) * Bump golang-ci lint to 1.61 ([#3112](#3112)) ([f23e085](f23e085)) * Secret Validation change ([#3111](#3111)) ([666630e](666630e)) ### 🐛 **Bug fixes:** * Fix parsing text in view, check parenthesis in ParseSchemaObjectIdentifierWithArguments ([#3102](#3102)) ([b0a67e6](b0a67e6)) * Try to reproduce 2679 and 3117 ([#3124](#3124)) ([ccdbc30](ccdbc30)) --- 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 - Mostly done task resource refactor - Sweepers test fixed (they always added errors even when they're nil) - Minor changes in SDK (find out the bug with alter task and right now we're only supporting upper-cased warehouse names; added that to the field description) - Acceptance tests - The enabled field is not a three-valued boolean because it created certain issues when updating the task. The task status is very important for their management and working, so I decided to have them as regular boolean values that are either true or false, nothing in between. ## Next prs - Apply comments from #3086 - Data source - Move some of the logic to SDK - Refactor SDK suspend root logic for tasks (and overall suspend logic in SDK/resource) - Examples, documentation, and migration guide - More test cases for complicated DAG structures to show the resource can handle more complex structures - Analyze non-deterministic test cases - Refactor task resuming in task resource (most likely with the use of defer) because currently, there may be cases that error can cause tasks to be not resumed. - Test and see how the DAG of tasks could be owned by a role other than the one created with Terraform (also with less privileges, only to run the task). - Make config without $$ escapes needed - Check if more helper functions from user resource could be used in task resource. - Check in one acceptance test why finalizer task in show_output is not set (is that Snowflake or mapping error). - More tests for calling (`as` field) - #3113 (comment) - Re-generate and list all the issues with asserts and models - check test tasks_gen_integration_test.go:937 (and see why it's non deterministic). - Support session paramters ## References * [CREATE TASK](https://docs.snowflake.com/en/sql-reference/sql/create-task)

- [x] Apply comments from #3086 - [x] Check if more helper functions from user resource could be used in task resource. - [ ] Apply comments from #3113 - [x] Cron #3113 (comment) - [ ] Resource logic #3113 (comment) - [ ] Refactor SDK suspend root logic for tasks (and overall suspend logic in SDK/resource) #3113 (comment) - [ ] Move some of the logic to SDK (if possible) - [ ] #3170 (comment) - [ ] Refactor task resuming in task resource (most likely with the use of defer) because currently, there may be cases that error can cause tasks to be not resumed. - [ ] Tests - [ ] External changes - #3113 (comment) - [ ] Add more complicated DAG structures to show the resource can handle more complex structures - [ ] Calling (`as` field) - #3113 (comment) - [ ] For showing how the DAG of tasks could be owned by a role other than the one created with Terraform (also with less privileges, only to run the task). - [ ] Check in one acceptance test why finalizer task in show_output is not set (is that Snowflake or mapping error). - [ ] Data source - [ ] Examples, documentation, and migration guide - [ ] Keep manually changed files after regeneration #3113 (comment) - [ ] Make config without $$ escapes needed - [ ] Support session paramters - [ ] Analyze non-deterministic test cases - [ ] Check test tasks_gen_integration_test.go:937 (and see why it's non deterministic). - [ ] Re-generate and list all the issues with asserts and models

sfc-gh-jcieslak added 5 commits September 18, 2024 10:19

wip

39c6bcf

wip

fff9b1e

Passing tests

110d00a

Added create or alter test

59d2fa4

wip

1224a7d

sfc-gh-jcieslak force-pushed the tasks-v1-readiness-sdk-part branch from 392ccae to 1224a7d Compare September 18, 2024 08:20

sfc-gh-jcieslak added 2 commits September 18, 2024 11:17

wip

56b338b

wip

e076055

sfc-gh-jcieslak marked this pull request as ready for review September 18, 2024 09:50

wip

5e808d1

sfc-gh-jmichalak requested changes Sep 19, 2024

View reviewed changes

sfc-gh-jcieslak added 2 commits September 20, 2024 13:06

changes after review

65ece76

changes after review

554fc90

lint fix

87638ea

sfc-gh-jmichalak previously approved these changes Sep 23, 2024

View reviewed changes

Changes after review

a783bec

sfc-gh-jcieslak dismissed sfc-gh-jmichalak’s stale review via a783bec September 23, 2024 12:26

Changes after review

fc5586e

sfc-gh-jmichalak approved these changes Sep 23, 2024

View reviewed changes

sfc-gh-jcieslak merged commit 0a77383 into main Sep 23, 2024
8 of 9 checks passed

sfc-gh-jcieslak deleted the tasks-v1-readiness-sdk-part branch September 23, 2024 14:03

snowflake-release-please bot mentioned this pull request Sep 23, 2024

chore(main): release 0.97.0 #3100

Merged

sfc-gh-asawicki reviewed Oct 1, 2024

View reviewed changes

sfc-gh-jcieslak mentioned this pull request Oct 4, 2024

feat: Task resource v1 readiness #3113

Merged

sfc-gh-jcieslak mentioned this pull request Nov 4, 2024

feat: Task resource v1 readiness part 2 #3170

Merged

23 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Tasks v1 readiness - SDK part #3086

feat: Tasks v1 readiness - SDK part #3086

sfc-gh-jcieslak commented Sep 18, 2024 •

edited

Loading

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 20, 2024

github-actions bot commented Sep 23, 2024

github-actions bot commented Sep 23, 2024

github-actions bot commented Sep 23, 2024

sfc-gh-asawicki Oct 1, 2024

sfc-gh-jcieslak Nov 4, 2024 •

edited

Loading

sfc-gh-asawicki Oct 1, 2024

sfc-gh-jcieslak Nov 4, 2024

sfc-gh-asawicki Oct 1, 2024

sfc-gh-jcieslak Nov 4, 2024

feat: Tasks v1 readiness - SDK part #3086

feat: Tasks v1 readiness - SDK part #3086

Conversation

sfc-gh-jcieslak commented Sep 18, 2024 • edited Loading

Changes

References

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 18, 2024

github-actions bot commented Sep 20, 2024

github-actions bot commented Sep 23, 2024

github-actions bot commented Sep 23, 2024

github-actions bot commented Sep 23, 2024

sfc-gh-asawicki Oct 1, 2024

Choose a reason for hiding this comment

sfc-gh-jcieslak Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

sfc-gh-asawicki Oct 1, 2024

Choose a reason for hiding this comment

sfc-gh-jcieslak Nov 4, 2024

Choose a reason for hiding this comment

sfc-gh-asawicki Oct 1, 2024

Choose a reason for hiding this comment

sfc-gh-jcieslak Nov 4, 2024

Choose a reason for hiding this comment

sfc-gh-jcieslak commented Sep 18, 2024 •

edited

Loading

sfc-gh-jcieslak Nov 4, 2024 •

edited

Loading