-
Notifications
You must be signed in to change notification settings - Fork 427
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 #3222
Merged
Merged
feat: Tasks v1 readiness #3222
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## 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
## Changes - Fixed the need to enclose `config` with $$ - Added full datasource support with tests - Documentation and examples (v1 candidates; added for resource and datasource) - Migration guide (now only for datasource) - Extracted schema for regular `in` filtering - Describe wasn't added on purpose, because it seems to mirror values from what we get from the SHOW command (can also add it, but for now it doesn't provide any benefits) ## List from previous prs - [ ] Apply comments #3170 - [ ] 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). - [x] Check in one acceptance test why finalizer task in show_output is not set (is that Snowflake or mapping error). - [x] Data source - [ ] Examples, documentation, and migration guide - [ ] Keep manually changed files after regeneration #3113 (comment) - [x] 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
# Conflicts: # pkg/acceptance/bettertestspoc/README.md # pkg/acceptance/bettertestspoc/assert/resourceassert/gen/resource_schema_def.go # pkg/acceptance/helpers/notification_integration_client.go # pkg/internal/collections/collection_helpers.go
- [x] Apply comments from #3113 - [x] Cron #3113 (comment) - [x] Resource logic #3113 (comment) - [x] Refactor SDK suspend root logic for tasks (and overall suspend logic in SDK/resource) #3113 (comment) - [x] Tests - [x] External changes - #3113 (comment) - [x] Add more complicated DAG structures to show the resource can handle more complex structures - [x] Calling (`as` field) - #3113 (comment) - [x] Check in one acceptance test why the finalizer task in show_output is not set (is that Snowflake or mapping error). - [x] Data source - [x] Examples, documentation, and migration guide - [x] Keep manually changed files after regeneration #3113 (comment) - [x] Make config without $$ escapes needed - [x] Support session parameters (I think it's already done, but check https://docs.snowflake.com/en/sql-reference/parameters#label-session-parameters) - [x] 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. - [x] Analyze non-deterministic test cases (All tests seem to be deterministic, only object_parameter resource tests are causing the AllParameters test to fail) - [x] Check test tasks_gen_integration_test.go:937 (and see why it's non-deterministic). - [x] Move some of the logic to SDK (if possible) - [x] #3170 (comment) - [x] Apply comments #3170 - [ ] Re-generate and list all the issues with asserts and models
# Conflicts: # pkg/resources/resource_monitor.go # pkg/resources/task.go
…1-readiness # Conflicts: # pkg/resources/task.go
sfc-gh-asawicki
approved these changes
Nov 25, 2024
Integration tests failure for d8701edaaaa077cc8495a59f0d208e659ce28ee0 |
sfc-gh-jmichalak
approved these changes
Nov 25, 2024
Integration tests failure for d8701edaaaa077cc8495a59f0d208e659ce28ee0 |
sfc-gh-jmichalak
pushed a commit
that referenced
this pull request
Nov 26, 2024
🤖 I have created a release *beep* *boop* --- ## [0.99.0](v0.98.0...v0.99.0) (2024-11-26) ### 🎉 **What's new:** * Add tags data source ([#3211](#3211)) ([8907d9d](8907d9d)) * Tag resource v1 ([#3197](#3197)) ([77b3bf0](77b3bf0)) * Tasks v1 readiness ([#3222](#3222)) ([e2284d9](e2284d9)) ### 🔧 **Misc** * Add support for usage tracking to data sources ([#3224](#3224)) ([8210bb8](8210bb8)) * Add usage tracking for the rest of the resources and fix views ([#3223](#3223)) ([231f653](231f653)) * Basic object tracking ([#3205](#3205)) ([1f0dc94](1f0dc94)) * basic object tracking part 2 ([#3214](#3214)) ([e44f2e1](e44f2e1)) * Improve tags integration tests ([#3193](#3193)) ([7736e0a](7736e0a)) * parser and secret tests ([#3192](#3192)) ([5ec9c86](5ec9c86)) * Storage integration with custom protocol ([#3213](#3213)) ([a3a44ae](a3a44ae)) * Unskip auth config tests ([#3180](#3180)) ([46ab142](46ab142)) ### 🐛 **Bug fixes:** * Small fixes and adjustments ([#3226](#3226)) ([9f67457](9f67457)) --- 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pr that collects all the changes to tasks from previous prs:
Note: look at the last commit, added very small changes to documentation and code (around 8 lines)