-
Notifications
You must be signed in to change notification settings - Fork 418
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: Tasks v1 readiness - SDK part #3086
Conversation
392ccae
to
1224a7d
Compare
Integration tests failure for 1224a7d7c203327688f6cc07481276983f8b9945 |
Integration tests failure for 56b338b6a902ea1743349a13e1bda715031fa9ef |
Integration tests failure for e076055b3e1213e5518abbf3e6a4f14d22f2d153 |
Integration tests failure for 5e808d1a232ee1c2cae7acf735092af6a20f01e1 |
func (t *TaskAssert) HasTaskRelations(expected sdk.TaskRelations) *TaskAssert { | ||
t.AddAssertion(func(t *testing.T, o *sdk.Task) error { | ||
t.Helper() | ||
if slices.EqualFunc(o.TaskRelations.Predecessors, expected.Predecessors, func(id sdk.SchemaObjectIdentifier, id2 sdk.SchemaObjectIdentifier) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about a different order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't really matter, we only care are about expected ids being there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but they can be returned in a different order. Please use ElementsMatch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how testify package works with our package, so I rewrote checks to be similar to the assert above (it asserts predecessors, but in the Predecessors field).
Integration tests failure for 554fc90c3931e8ac77d5121095616a83633d339e |
Integration tests failure for 87638ea7c7be33c93a211ea5a26e43d5b752bbc9 |
Integration tests failure for fc5586ec5c63d5bae081066e85bf8161387b169b |
Integration tests failure for a783bec3eea98546233ae6dee44e012cc08b9020 |
return t | ||
} | ||
|
||
func (t *TaskAssert) HasPredecessors(ids ...sdk.SchemaObjectIdentifier) *TaskAssert { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
@@ -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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why we get rid of the helper func from SDK?
@@ -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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
🤖 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
References