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

Jump to bottom

feat: Add identifier parsers #2957

Merged
merged 12 commits into from
Aug 5, 2024
Merged

feat: Add identifier parsers #2957

merged 12 commits into from
Aug 5, 2024

Conversation

sfc-gh-jcieslak
Copy link
Collaborator

@sfc-gh-jcieslak sfc-gh-jcieslak commented Jul 24, 2024 •
edited
Loading

Changes:

  • Add and test identifier parsers for object and account identifiers with different separators (only dot was added as identifier parser, description provided below).

The encoder/parser for resource identifiers was in the end implemented as simple string join/split by pipe, because of the following:

  • Identifiers that have only one part that is represented by the sdk.ObjectIdentifier should be encoded as FullyQualifiedName and parsed with one of the newly introduced parsers. This should improve a bit the support for tools that generate import commands (or import blocks) because the resource identifier is the same as the Snowflake one.
  • More complicated identifiers having more parts should use the provided encoder/parser to create or parse resource identifiers into parts. This could cause problems for things that may contain pipe, but we should generally avoid it or document it (can be done in the usage part of identifier rework). Also, CSV reader/writer cannot be used for resource identifiers, because this would make identifiers more complex. For example, a fully qualified identifier with quotes would have to be imported as terraform import snowflake_table.test_table '"""database_name"".""schema_name"".table_name"""'. To have this as an identifier would also mean creating a state upgrader for every resource (or general one that could be reused) that would transform the identifiers into CSV-compliant ones.

Notes:

  • Implementation with the use of identifeirParsingFunc was added because initially, I had two parsers (one with dot as csv.Comma and the other one with pipe) and later on removed the one with the pipe as csv.Comma (I can merge the unexported parsers with exported ones or leave it as is if we would like to introduce another internal parser).
  • ExternalObjectIdentifier and AccountIdentifier parsers were created with certain assumptions (described in code).

Test Plan

  • unit tests

References

  • SNOW-1495051

@sfc-gh-jcieslak sfc-gh-jcieslak force-pushed the implement-a-new-identifier-parsing-function branch 2 times, most recently from ca4f1f0 to 236ebf6 Compare July 24, 2024 13:21
@sfc-gh-jcieslak sfc-gh-jcieslak force-pushed the implement-a-new-identifier-parsing-function branch from 236ebf6 to 5602adb Compare July 24, 2024 13:21
@github-actions GitHub Actions
Copy link

Integration tests failure for ca4f1f0d7fa5bf532b3203e04033d6a73f966a46

@github-actions GitHub Actions
Copy link

Integration tests failure for 393d34cd7221cb8db1397d7236b743068ed6d630

@github-actions GitHub Actions
Copy link

Integration tests failure for 8d9180b369da76e7f75d8e7b041dc8f59beb8e18

@github-actions GitHub Actions
Copy link

Integration tests failure for 236ebf62e1a522b30be0dfe91f88ab7b81d18cf3

@github-actions GitHub Actions
Copy link

Integration tests failure for 5602adbb7d7a8155cbfb30f6e3296da2835a6196

sfc-gh-jmichalak
sfc-gh-jmichalak requested changes Jul 25, 2024
View reviewed changes
pkg/helpers/identifier_string_parser.go Outdated Show resolved Hide resolved
pkg/helpers/identifier_string_parser.go Outdated Show resolved Hide resolved
pkg/helpers/identifier_string_parser_test.go Outdated Show resolved Hide resolved
pkg/helpers/identifier_string_parser_test.go Outdated Show resolved Hide resolved
sfc-gh-asawicki
sfc-gh-asawicki requested changes Jul 25, 2024
View reviewed changes
pkg/helpers/identifier_string_parser.go Outdated Show resolved Hide resolved
pkg/helpers/identifier_string_parser.go Outdated Show resolved Hide resolved
pkg/helpers/identifier_string_parser_test.go Outdated Show resolved Hide resolved
pkg/helpers/identifier_string_parser_test.go Outdated Show resolved Hide resolved
pkg/helpers/identifier_string_parser_test.go Outdated Show resolved Hide resolved
pkg/helpers/identifier_string_parser.go Outdated Show resolved Hide resolved
pkg/helpers/identifier_string_parser.go Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

Integration tests failure for 7b9bf829eb4ca919d0f2d4e29401c2a9522779df

@github-actions GitHub Actions
Copy link

Integration tests failure for 17875addf91879fb0407a8ec7428b089c81fb0e4

@sfc-gh-jcieslak sfc-gh-jcieslak force-pushed the implement-a-new-identifier-parsing-function branch from 6b74a9f to 1698ed5 Compare July 30, 2024 09:55
@github-actions GitHub Actions
Copy link

Integration tests failure for 6b74a9f56434735cdb17645e97d8a4d34193cd6e

@github-actions GitHub Actions
Copy link

Integration tests failure for 1698ed595a0ac0f9ea939de2e0863d02ad54a39c

@github-actions GitHub Actions
Copy link

Integration tests failure for b80b6cdb3fabd359bb2cf872a3044b20083f5564

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review July 31, 2024 07:28
sfc-gh-jmichalak
sfc-gh-jmichalak previously approved these changes Jul 31, 2024
View reviewed changes
pkg/resources/secondary_database_acceptance_test.go Outdated Show resolved Hide resolved
pkg/helpers/resource_identifier_helpers.go Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

Integration tests failure for b71b1748dc9506b923c956908e8d810a93c7f085

sfc-gh-asawicki
sfc-gh-asawicki requested changes Aug 1, 2024
View reviewed changes
pkg/resources/shared_database_acceptance_test.go Outdated Show resolved Hide resolved
pkg/sdk/identifier_helpers.go Outdated Show resolved Hide resolved
@sfc-gh-jcieslak sfc-gh-jcieslak force-pushed the implement-a-new-identifier-parsing-function branch from 0a80df9 to cd957ff Compare August 2, 2024 10:32
@sfc-gh-jcieslak sfc-gh-jcieslak force-pushed the implement-a-new-identifier-parsing-function branch from cd957ff to 98cb9b8 Compare August 2, 2024 10:39
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 2, 2024

Integration tests failure for 0a80df97845a740248927d51b92d35fb7f227249

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 2, 2024

Integration tests failure for cd957ff9e6fb218441d6e8659ac9034e17aa5a11

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 2, 2024

Integration tests failure for 98cb9b81deeeaad29632888f367981a977e8dea3

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review August 5, 2024 08:33
@sfc-gh-jcieslak sfc-gh-jcieslak merged commit 824ec52 into main Aug 5, 2024
8 of 9 checks passed
@sfc-gh-jcieslak sfc-gh-jcieslak deleted the implement-a-new-identifier-parsing-function branch August 5, 2024 08:43
@snowflake-release-please snowflake-release-please bot mentioned this pull request Aug 5, 2024
Merged
sfc-gh-jcieslak pushed a commit that referenced this pull request Sep 4, 2024
@snowflake-release-please
chore(main): release 0.95.0 (#2977)
cd1636c 
🤖 I have created a release *beep* *boop*
---


##
[0.95.0](v0.94.1...v0.95.0)
(2024-09-04)


### 🎉 **What's new:**

* Add change_tracking, row access policy and aggregation policy to views
([#2988](#2988))
([1f88bb1](1f88bb1))
* Add fully_qualified_name to all resources
([#2990](#2990))
([1b0462f](1b0462f))
* Add identifier parsers
([#2957](#2957))
([824ec52](824ec52))
* Add identifier with arguments
([#2979](#2979))
([00ae1c5](00ae1c5))
* Add timeouts block to cortex
([#3004](#3004))
([34d764b](34d764b))
* Add user parameters to resource
([#2968](#2968))
([f4ae380](f4ae380))
* Conclude user rework
([#3036](#3036))
([23e4625](23e4625))
* database role v1 readiness
([#3014](#3014))
([c4db255](c4db255))
* Identifier with arguments for procedure and external function
([#2987](#2987))
([f13cc5c](f13cc5c))
* Rework user resource
([#3026](#3026))
([bde2638](bde2638)),
closes
[#1572](#1572)
* Rework users datasource
([#3030](#3030))
([751239b](751239b)),
closes
[#2902](#2902)
* Upgrade view sdk
([#2969](#2969))
([ef2d50a](ef2d50a))
* View rework part 2
([#3021](#3021))
([e05377d](e05377d))
* View rework part 3
([#3023](#3023))
([195b41c](195b41c))


### 🔧 **Misc**

* Add annotation about fully_qualified_name and fix handling granteeName
([#3009](#3009))
([94e6345](94e6345))
* Apply identifier conventions
([#2996](#2996))
([5cbea84](5cbea84))
* apply identifier conventions to grants
([#3008](#3008))
([d7780ae](d7780ae))
* Clean collection utils
([#3028](#3028))
([426ddb1](426ddb1))
* Clean old assertions
([#3029](#3029))
([ad657eb](ad657eb))
* Conclude identifiers rework
([#3011](#3011))
([c1b53f3](c1b53f3))
* Improve user test and add manual test for user default database and
role
([#3035](#3035))
([6cb0b4e](6cb0b4e))
* Use new identifier with arguments in function, external function and
procedure grants
([#3002](#3002))
([5053f8b](5053f8b))
* User improvements
([#3034](#3034))
([65b64d7](65b64d7))


### 🐛 **Bug fixes:**

* database tests and introduce a new parameter
([#2981](#2981))
([3bae7f6](3bae7f6))
* Fix custom diffs for fields with diff supression
([#3032](#3032))
([2499602](2499602))
* Fix default secondary roles after BCR 2024_07
([#3040](#3040))
([2ca465a](2ca465a)),
closes
[#3038](#3038)
* Fix issues 2972 and 3007
([#3020](#3020))
([1772387](1772387))
* Fix known user resource issues
([#3013](#3013))
([a5dfeac](a5dfeac))
* identifier issues
([#2998](#2998))
([6fb76b7](6fb76b7))
* minor issues
([#3027](#3027))
([467b06e](467b06e)),
closes
[#3015](#3015)
[#2807](#2807)
[#3025](#3025)
* Nuke users
([#2971](#2971))
([0d90cc9](0d90cc9))

---
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
Reviewers

@sfc-gh-jmichalak sfc-gh-jmichalak sfc-gh-jmichalak approved these changes

@sfc-gh-asawicki sfc-gh-asawicki sfc-gh-asawicki approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-jcieslak @sfc-gh-asawicki @sfc-gh-jmichalak