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: Identifier with arguments for procedure and external function #2987

Merged
merged 12 commits into from
Aug 20, 2024
Merged

feat: Identifier with arguments for procedure and external function #2987

merged 12 commits into from
Aug 20, 2024

Conversation

sfc-gh-jcieslak
Copy link
Collaborator

@sfc-gh-jcieslak sfc-gh-jcieslak commented Aug 9, 2024 •
edited
Loading

Changes

  • New identifier with arguments used in procedures and external functions.
  • Added state upgrader for all 3 resources (the previous identifier wasn't outputting empty parentheses for no args).
  • Added tests to ensure smooth migration.
  • Minor fixes (not much since they'll be refactored later on to prepare them for v1); most of the changes come from re-generation of _def.go definitions.
  • Added case for argument parsing arguments with default values.

@sfc-gh-jcieslak
wip
4d8b845 
save (testing on function)

save
@sfc-gh-jcieslak
wip
14067c5
@sfc-gh-jcieslak
wip
31521e7
@sfc-gh-jcieslak
wip
f0b3fe2
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 9, 2024

Integration tests failure for f0b3fe276f081012ce38ff359b8ffc5cf6d9f89d

@github-actions GitHub Actions
Copy link

Integration tests failure for 9c2f89a282f3e93c6acc99ccdaa009793ab3e16a

@github-actions GitHub Actions
Copy link

Integration tests failure for 116bb2cb7a4d98e9f05f9a58e502424bc78ad7bd

@github-actions GitHub Actions
Copy link

Integration tests failure for a88eeae59bf5f8790a1103539aa832920f3115d2

sfc-gh-jmichalak
sfc-gh-jmichalak previously approved these changes Aug 13, 2024
View reviewed changes
pkg/sdk/procedures_impl_gen.go Show resolved Hide resolved
pkg/sdk/testint/procedures_integration_test.go Outdated Show resolved Hide resolved
pkg/sdk/testint/external_functions_integration_test.go Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

Integration tests failure for d4b2bb298c153866a8572e67064eba519f85008e

@sfc-gh-jcieslak
Merge branch 'refs/heads/main' into identifier-with-arguments-for-pro…
dc6317b 
…cedure-and-external-function

# Conflicts:
#	pkg/resources/procedure.go
@github-actions GitHub Actions
Copy link

Integration tests failure for dc6317b876c6e6fde315aee95259c8fed1cf1db4

sfc-gh-asawicki
sfc-gh-asawicki previously approved these changes Aug 16, 2024
View reviewed changes
pkg/resources/external_function.go Show resolved Hide resolved
pkg/resources/external_function_acceptance_test.go Show resolved Hide resolved
pkg/sdk/identifier_helpers.go Show resolved Hide resolved
pkg/sdk/identifier_parsers.go Show resolved Hide resolved
pkg/sdk/testint/external_functions_integration_test.go Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

Integration tests failure for b26d5fce58069e4a70b82d659243efdf673d4749

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review August 16, 2024 13:04
@sfc-gh-jmichalak sfc-gh-jmichalak mentioned this pull request Aug 16, 2024
Merged
3 tasks
@sfc-gh-jcieslak sfc-gh-jcieslak merged commit f13cc5c into main Aug 20, 2024
8 of 9 checks passed
@sfc-gh-jcieslak sfc-gh-jcieslak deleted the identifier-with-arguments-for-procedure-and-external-function branch August 20, 2024 08:49
@github-actions GitHub Actions
Copy link

Integration tests cancelled for 53033bb8fac5e4d94933da43060089f8f55837db

@snowflake-release-please snowflake-release-please bot mentioned this pull request Aug 20, 2024
Merged
sfc-gh-jmichalak added a commit that referenced this pull request Aug 20, 2024
@sfc-gh-jmichalak @sfc-gh-jcieslak
chore: Use new identifier with arguments in function, external functi…
5053f8b 
…on and procedure grants (#3002)

<!-- Feel free to delete comments as you fill this in -->
- handle unusual identifiers returned in SHOW GRANTS 
- handle function, external function and procedure identifiers in
ownership and privilege grants properly
- add tests for these objects with and without arguments
- remove IsValidIdentifier from validations - it doesn't work for ids
with arguments

Note: base branch of this PR is
`identifier-with-arguments-for-procedure-and-external-function` and
should be merged soon in
#2987
 <!-- summary of changes -->

TODO:
- handle `granteeName` for shares in next PR (instead of using a
fallback in `convert`), parse the ID outside the function when we know
SHOW GRANT request details, so we can parse based on that.

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [x] acceptance tests
* [x] integration tests
* [x] unit tests

## References
<!-- issues documentation links, etc  -->
https://docs.snowflake.com/en/sql-reference/sql/grant-privilege and
other grant docs

---------

Co-authored-by: Jan Cieślak <jan.cieslak@snowflake.com>
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