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.

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

Reuse Snowflake connection throughout Streamlit app #99

Merged
merged 5 commits into from
Jul 29, 2024

Conversation

sfc-gh-cnivera
Copy link
Collaborator

@sfc-gh-cnivera sfc-gh-cnivera commented Jul 26, 2024

Resolves #90, #97

In the current version of the admin app, we manually instantiate a SnowflakeConnection multiple times throughout the codebase. This isn't particularly a problem when the user is using standard username/pw credentials; however, when using MFA or external-browser authentication, this causes multiple pings/browser windows to pop up, which interrupts the UX significantly.

I've modified the Streamlit app to instantiate a single top-level Snowflake connection and cached it via st.cache_resource. With Streamlit's caching, whenever the decorated function is run for the very first time in the lifetime of the app, the connection is created and stored in cache. Whenever the function is invoked again, the connection is retrieved from cache instead of instantiated from scratch. See https://docs.streamlit.io/develop/concepts/architecture/caching#stcache_resource for more details.

I also had to make some changes to the underlying model generation/validation logic so that a connection can optionally be passed in instead of instantiating new connections.

Testing

Use the app with a user/account that uses the externalbrowser authenticator type. Verify that you see a single auth popup on app start, and only once. Verify that you do not see any more popups while using the app.


for vq in model.verified_queries:
logger.info(f"Checking verified queries for: {vq.question}")
with connector.connect(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think setting the db/schema in this connection was a no-op since this was outside of the for table in ... statement above, so I just removed it.

@sfc-gh-cnivera sfc-gh-cnivera force-pushed the cnivera/reuse-connection branch from 5eac5fe to b94dcc7 Compare July 26, 2024 22:38
@sfc-gh-cnivera sfc-gh-cnivera linked an issue Jul 26, 2024 that may be closed by this pull request
@sfc-gh-cnivera sfc-gh-cnivera marked this pull request as ready for review July 26, 2024 23:03
Copy link
Collaborator

@sfc-gh-yayin sfc-gh-yayin left a comment

Choose a reason for hiding this comment

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

Tested it locally, lgtm

@sfc-gh-cnivera sfc-gh-cnivera merged commit d34ee3a into main Jul 29, 2024
3 checks passed
@sfc-gh-cnivera sfc-gh-cnivera deleted the cnivera/reuse-connection branch July 29, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate credentials on startup Streamlit App: Cache Snowflake connection
2 participants