feat: use SDK in schema resource and datasource #2082
Conversation
|
Integration tests failure for 7504035d47a34d65c608f530c94d68f16d69e3ed |
sfc-gh-jcieslak
force-pushed
the
use-schemas-from-sdk
branch
from
7504035
to
fbd0946
Compare
|
Integration tests failure for fbd0946207cc07f5c81aac42e393ec0f97c54765 |
| } | ||
| return sdk.NewDatabaseObjectIdentifier(v["database"].(string), v["name"].(string)) | ||
| } | ||
| return sdk.NewAccountObjectIdentifier(v["name"].(string)) |
There was a problem hiding this comment.
Not sure about this. I think tags are database or database+schema objects and without them it cannot be created / retrieved (unless there's active database or database+schema in the client session), so I should return error here. What do you think ?
There was a problem hiding this comment.
Hmmm, I think tags are indeed on schema level: https://docs.snowflake.com/en/user-guide/object-tagging#what-is-a-tag and I guess this should be the only option here.
Both schema and database are required in schema resource definition, so it is safe to assume they will be present here.
@scottwinkler, can you also chip in?
There was a problem hiding this comment.
tags are indeed schema level objects. although it might appear like you could create a tag without a database or schema, since you can do create tag "my_tag", this uses whatever your current database and schema are set to in the given session
|
Integration tests failure for 515366df0fc207157436c88cfee01b04a97bc21e |
|
Integration tests success for b7ae972fcad1aaf9a7f6e69770ee1a6520faee1f |
|
Integration tests failure for 64e177215e2115bd2a74d699ff98db85d51684e7 |
sfc-gh-jcieslak
force-pushed
the
use-schemas-from-sdk
branch
from
64e1772
to
9a43790
Compare
|
Integration tests failure for 9a437900ca83e6d55aab906e4edbbbc8a493f0ce |
1 similar comment
|
Integration tests failure for 9a437900ca83e6d55aab906e4edbbbc8a493f0ce |
| } | ||
| return sdk.NewDatabaseObjectIdentifier(v["database"].(string), v["name"].(string)) | ||
| } | ||
| return sdk.NewAccountObjectIdentifier(v["name"].(string)) |
There was a problem hiding this comment.
Hmmm, I think tags are indeed on schema level: https://docs.snowflake.com/en/user-guide/object-tagging#what-is-a-tag and I guess this should be the only option here.
Both schema and database are required in schema resource definition, so it is safe to assume they will be present here.
@scottwinkler, can you also chip in?
| return sdk.NewAccountObjectIdentifier(v["name"].(string)) | ||
| } | ||
|
|
||
| func getPropertyTags(d *schema.ResourceData, key string) []sdk.TagAssociation { |
There was a problem hiding this comment.
What was the plan for tags and tags associations @scottwinkler? I can see that now they are supported in two places: for some resource, like schema here, they have "tag": tagReferenceSchema specified, but tagReferenceSchema has Deprecated: "Use the 'snowflake_tag_association' resource instead.".
So I believe that for now we want to support both options, and with V1 we cut out this deprecated option, right?
There was a problem hiding this comment.
well i think it is a similar problem to the snowflake_parameters vs having the same attribute be on a resource. We also have to weigh the downside of basically doubling the number of resources, if customers want to tag every resource. i didn't like the implementation of tag blocks previously, because it was difficult to know when the order changed, and also it made for really long configuration code, but I could see maybe adding them back later with a slightly different syntax, e.g.:
snowflake_database "d" {
....
tags = [
{
tag_name = ""
tag_value = ""
},
{
tag_name = ""
tag_value = ""
},
...
]
}
That being said, if we add tags back in it would be problematic for people who use the existing tag association resource. They would need to set ignore_changes = ["tags"] on every resource which uses tags so as not dissociate every tag they had set. https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle#ignore_changes. At the time it was too complicated to have a tags block on every resource, which is why it was made into its own resource. Now that we have a more stable API, I'm not sure its beneficial anymore.
sfc-gh-jcieslak
force-pushed
the
use-schemas-from-sdk
branch
from
233f3e7
to
c030324
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
sfc-gh-jcieslak
force-pushed
the
use-schemas-from-sdk
branch
from
c030324
to
32b8f71
Compare
|
Integration tests success for c0303245a7b58fa2f0a11ca7116d20ae62929300 |
|
Integration tests failure for 233f3e763a4ef36c1014318ca29f0a1db95254ad |
|
Integration tests failure for 32b8f71da47fcdefe7abb83da5477c21a045de29 |
|
Integration tests success for 32b8f71da47fcdefe7abb83da5477c21a045de29 |
sfc-gh-jcieslak
force-pushed
the
use-schemas-from-sdk
branch
from
32b8f71
to
cc75134
Compare
|
Integration tests failure for cc751347d502d597809e3209151cede438c20e8d |
1 similar comment
|
Integration tests failure for cc751347d502d597809e3209151cede438c20e8d |
| } | ||
| return sdk.NewDatabaseObjectIdentifier(v["database"].(string), v["name"].(string)) | ||
| } | ||
| return sdk.NewAccountObjectIdentifier(v["name"].(string)) |
There was a problem hiding this comment.
tags are indeed schema level objects. although it might appear like you could create a tag without a database or schema, since you can do create tag "my_tag", this uses whatever your current database and schema are set to in the given session
| return sdk.NewAccountObjectIdentifier(v["name"].(string)) | ||
| } | ||
|
|
||
| func getPropertyTags(d *schema.ResourceData, key string) []sdk.TagAssociation { |
There was a problem hiding this comment.
well i think it is a similar problem to the snowflake_parameters vs having the same attribute be on a resource. We also have to weigh the downside of basically doubling the number of resources, if customers want to tag every resource. i didn't like the implementation of tag blocks previously, because it was difficult to know when the order changed, and also it made for really long configuration code, but I could see maybe adding them back later with a slightly different syntax, e.g.:
snowflake_database "d" {
....
tags = [
{
tag_name = ""
tag_value = ""
},
{
tag_name = ""
tag_value = ""
},
...
]
}
That being said, if we add tags back in it would be problematic for people who use the existing tag association resource. They would need to set ignore_changes = ["tags"] on every resource which uses tags so as not dissociate every tag they had set. https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle#ignore_changes. At the time it was too complicated to have a tags block on every resource, which is why it was made into its own resource. Now that we have a more stable API, I'm not sure its beneficial anymore.
|
Integration tests success for cc751347d502d597809e3209151cede438c20e8d |
sfc-gh-jcieslak
force-pushed
the
use-schemas-from-sdk
branch
from
cc75134
to
63747da
Compare
|
Integration tests failure for 63747da90ba1bbeb404acb477d8c0012c44c9a43 |
sfc-gh-jcieslak
force-pushed
the
use-schemas-from-sdk
branch
from
63747da
to
02ee64c
Compare
|
Integration tests failure for 02ee64c603ddb908d3a7548e973980bf3139d559 |
sfc-gh-jcieslak
force-pushed
the
use-schemas-from-sdk
branch
from
02ee64c
to
dbd3a55
Compare
|
Integration tests failure for dbd3a5528851cf3c0936942f5c1618f7c3528521 |
sfc-gh-jcieslak
force-pushed
the
use-schemas-from-sdk
branch
from
dbd3a55
to
b1c0c88
Compare
|
Integration tests success for b1c0c88c91f1ed6274edbbee511d2e1b6766cba7 |
No description provided.