-
Notifications
You must be signed in to change notification settings - Fork 79
SNOW-2206349 Fix Streamlit Entity Grants #2615
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
Merged
sfc-gh-daniszewski
merged 14 commits into
main
from
daniszewski-SNOW-2206349-fix-streamlit-grants
Sep 26, 2025
Merged
SNOW-2206349 Fix Streamlit Entity Grants #2615
sfc-gh-daniszewski
merged 14 commits into
main
from
daniszewski-SNOW-2206349-fix-streamlit-grants
Sep 26, 2025
Conversation
This file contains hidden or 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
- Removes SECURITYADMIN privilege requirement for CI - Uses existing PUBLIC role to test grants functionality - Simplifies test flow while maintaining functionality validation
- Remove create_test_role() and cleanup_test_role() methods - These methods required SECURITYADMIN privileges and were causing CI failures - No longer needed since tests now use existing PUBLIC role
- test_role is specifically created for integration tests in CI environment - Avoids potential permission restrictions with PUBLIC role in CI - Follows standard integration test pattern from account setup
- Remove hardcoded ACCOUNTADMIN role switching in verify_grants_applied - Simplify verification by not switching back to original role - Fixes CI error: 'ACCOUNTADMIN role is not assigned to executing user' - Tests now pass locally with proper role handling
- Remove verify_grants_applied calls from integration tests - Tests still verify grants are applied during deployment - Avoids TEST_ROLE assignment issues in CI environment - More robust approach that focuses on core functionality - Both grants tests now pass cleanly
- Get current role from session instead of hardcoding role names - Ensures grants are validated with a role the CI user actually has - Tests now properly validate grants functionality in any environment - Both grants tests pass with full validation enabled - More robust than hardcoded role approaches
- Replace dynamic role discovery with snowflake_session.role - CI environment uses consistent static role configuration - Cleaner and more predictable than SQL queries - Maintains full grants validation functionality - Both tests still pass with proper validation
- Remove explanatory comments that don't add value - Code is self-explanatory without extra commentary - Cleaner and more concise implementation
sfc-gh-jwilkowski
previously approved these changes
Sep 26, 2025
sfc-gh-jwilkowski
approved these changes
Sep 26, 2025
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.
Pre-review checklist
Changes description
Grant privileges defined in
snowflakey.yml
after deploying StreamlitCloses #2491