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.

Sign up for GitHub

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: Task resource v1 readiness #3113

Merged
merged 14 commits into from
Nov 5, 2024
Merged

feat: Task resource v1 readiness #3113

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
38b8550
wip
sfc-gh-jcieslak Sep 25, 2024
6837bc5
wip
sfc-gh-jcieslak Sep 30, 2024
c2f707d
wip
sfc-gh-jcieslak Sep 30, 2024
6843e03
wip
sfc-gh-jcieslak Oct 1, 2024
12eadb0
wip
sfc-gh-jcieslak Oct 3, 2024
28bf6a5
wip
sfc-gh-jcieslak Oct 3, 2024
62adccb
wip
sfc-gh-jcieslak Oct 4, 2024
bba9569
Merge branch 'refs/heads/main' into task-resource-v1-readiness
sfc-gh-jcieslak Oct 11, 2024
80ca12e
wip
sfc-gh-jcieslak Oct 14, 2024
f0514bc
docs re-generated
sfc-gh-jcieslak Oct 14, 2024
ee9583d
Added test that proves the issue
sfc-gh-jcieslak Oct 14, 2024
90404b9
Merge branch 'refs/heads/main' into task-resource-v1-readiness
sfc-gh-jcieslak Nov 4, 2024
012fb18
Resolve merge conflicts
sfc-gh-jcieslak Nov 4, 2024
90d7046
Resolve merge conflicts and fix failing tests
sfc-gh-jcieslak Nov 4, 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: 2 additions & 2 deletions docs/resources/resource_monitor.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
page_title: "snowflake_resource_monitor Resource - terraform-provider-snowflake"
subcategory: ""
description: |-

Resource used to manage resource monitor objects. For more information, check resource monitor documentation https://docs.snowflake.com/en/user-guide/resource-monitors.
---

!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0950--v0960) to use it.
Expand All @@ -15,7 +15,7 @@ description: |-

# snowflake_resource_monitor (Resource)


Resource used to manage resource monitor objects. For more information, check [resource monitor documentation](https://docs.snowflake.com/en/user-guide/resource-monitors).

## Example Usage

Expand Down
884 changes: 870 additions & 14 deletions docs/resources/task.md
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions pkg/acceptance/bettertestspoc/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,3 +349,5 @@ func (w *WarehouseDatasourceShowOutputAssert) IsEmpty() {
The error is a result of both things:
1. Lists of objects are partially generated, and only parameter name is generated in some functions (the type has to be added manually).
2. `testing` is a package name that makes Go think that we want to have unnamed parameter there, but we just didn't generate the type for that field in the function argument.
- Additional asserts for sets and lists that wouldn't rely on the order of items saved to the state (SNOW-1706544)

22 changes: 17 additions & 5 deletions pkg/acceptance/bettertestspoc/assert/objectassert/task_snowflake_gen.go
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion pkg/acceptance/bettertestspoc/assert/objectparametersassert/task_parameters_snowflake_gen.go
View file
Open in desktop

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

4 changes: 4 additions & 0 deletions pkg/acceptance/bettertestspoc/assert/resourceassert/gen/resource_schema_def.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,8 @@ var allResourceSchemaDefs = []ResourceSchemaDef{
name: "MaskingPolicy",
schema: resources.MaskingPolicy().Schema,
},
{
name: "Task",
schema: resources.Task().Schema,
},
}
17 changes: 17 additions & 0 deletions pkg/acceptance/bettertestspoc/assert/resourceassert/task_resource_ext.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package resourceassert

import (
"fmt"

Check failure on line 4 in pkg/acceptance/bettertestspoc/assert/resourceassert/task_resource_ext.go

View workflow job for this annotation

GitHub Actions / reviewdog 

[golangci] reported by reviewdog 🐶
File is not `gofumpt`-ed (gofumpt)

Raw Output:
pkg/acceptance/bettertestspoc/assert/resourceassert/task_resource_ext.go:4: File is not `gofumpt`-ed (gofumpt)
	"fmt"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"

Check failure on line 5 in pkg/acceptance/bettertestspoc/assert/resourceassert/task_resource_ext.go

View workflow job for this annotation

GitHub Actions / reviewdog 

[golangci] reported by reviewdog 🐶
File is not `goimports`-ed (goimports)

Raw Output:
pkg/acceptance/bettertestspoc/assert/resourceassert/task_resource_ext.go:5: File is not `goimports`-ed (goimports)
	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"strconv"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert"
)

func (t *TaskResourceAssert) HasAfterIds(ids ...sdk.SchemaObjectIdentifier) *TaskResourceAssert {
t.AddAssertion(assert.ValueSet("after.#", strconv.FormatInt(int64(len(ids)), 10)))
for i, id := range ids {
t.AddAssertion(assert.ValueSet(fmt.Sprintf("after.%d", i), id.FullyQualifiedName()))
Copy link
Collaborator

Choose a reason for hiding this comment

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

The order shouldn't matter here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

SHouldn't because the assertion does not have InOrder in name or because Snowflake ordering for alter in non-deterministic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, but that should be resolved in newly added TODO for asserts to generate or create a helper for creating asserts for list with non-deterministic order. Right now, this one work (despite being reliant on order) and I would leave it as is.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's at least add InOrder suffix to the name of the function then (may be added in the next PR)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we discussed this in one of the previous PRs. Snowflake does not guarantee item order, so in general we should compare ID collections, not one by one. For cases where the order matters, we should add a comment about that.

Copy link
Collaborator Author

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

Choose a reason for hiding this comment

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

Addressed in: For now, renamed to have InOrder suffix, the limitation of order was added to assert improvements some time ago (it should be in main or in this pr.)

}
return t
}
Loading
Loading