Fix a regression with snowflake connection being closed#13665
Fix a regression with snowflake connection being closed#13665lukasmasuch merged 1 commit intodevelopfrom
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ PR preview is ready!
|
|
@cursor review |
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where Snowflake connections fail with a "Connection is closed" error after the connection has been evicted from the cache. The bug was introduced when on_release callbacks were added to st.cache_resource, which calls close() on evicted connections.
Changes:
- Fixed
BaseSnowflakeConnection.close()to reset_raw_instancetoNoneafter closing the connection - Added unit tests to verify the fix and prevent regressions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
lib/streamlit/connections/snowflake_connection.py |
Added self._raw_instance = None after closing the connection to ensure subsequent access creates a new connection instead of returning the closed one |
lib/tests/streamlit/connections/snowflake_connection_test.py |
Added TestSnowflakeConnectionClose test class with comprehensive tests covering the close behavior and edge cases |
SummaryThis PR fixes a regression in The fix is minimal and targeted: adding Code QualityThe code change is clean and follows existing patterns in the codebase:
Test CoverageThe tests are well-written and follow the Python unit test guidelines:
Minor observation: The tests could include a negative assertion to verify that a closed connection is NOT returned after Backwards CompatibilityThis change is fully backwards compatible:
Security & Risk
One consideration: Snowpark Sessions created via RecommendationsNo blocking issues found. The PR is ready for merge. Optional improvements (non-blocking):
def close(self) -> None:
"""Closes the underlying Snowflake connection."""
if self._raw_instance is not None:
self._raw_instance.close()
# Reset so _instance property creates a new connection on next access
self._raw_instance = None
VerdictAPPROVED: This is a well-implemented, minimal bug fix that correctly resolves a regression in Snowflake connection handling. The fix follows existing patterns in the codebase, and the tests adequately cover both the primary fix and edge cases. This is an automated AI review using |
📈 Frontend coverage change detectedThe frontend unit test (vitest) coverage has increased by 0.0500%
🎉 Great job on improving test coverage! |
## Describe your changes
Fixes an issue with Snowflake connections not getting re-initialized
after having been closed.
<details>
<summary>Claude issue analysis</summary>
# Issue Report: Snowflake Connection "Connection is closed" Error
## Summary
After recent PRs adding `on_release` to `st.cache_resource` and
session-scoped connection support, users may encounter a
`snowflake.connector.errors.DatabaseError: 250002 (08003): Connection is
closed` error when using `st.connection("snowflake")` with cached data
queries.
## Related PRs
- **PR #13439**: Add `on_release` to `st.cache_resource`
- **PR #13482**: Add session scoping to caches
- **PR #13538**: Add `SnowflakeCallersRightsConnection`
- **PR #13506**: Add session-scoped connection support
## Error Details
```
Traceback (most recent call last):
File ".../streamlit/runtime/scriptrunner/exec_code.py", line 129, in exec_func_with_error_handling
result = func()
...
File ".../snowflake/snowpark/_internal/server_connection.py", line 205, in _cursor
self._thread_store.cursor = self._conn.cursor()
File ".../snowflake/connector/connection.py", line 1270, in cursor
Error.errorhandler_wrapper(...)
snowflake.connector.errors.DatabaseError: 250002 (08003): Connection is closed
```
## Root Cause Analysis
### Background
The PRs mentioned added important functionality:
1. **PR #13439** added the `on_release` callback to `st.cache_resource`,
which is called when cache entries are evicted
2. **PR #13506** modified `connection_factory.py` to use this
`on_release` callback to call `connection.close()` when a connection is
evicted from the cache
This is the relevant code in `connection_factory.py`:
```python
def on_release_wrapped(connection: ConnectionClass) -> None:
connection.close()
__create_connection = cache_resource(
max_entries=max_entries,
show_spinner="Running `st.connection(...)`.",
ttl=ttl,
scope=scope,
on_release=on_release_wrapped, # Calls close() when evicted
)(__create_connection)
```
### The Bug
In `BaseSnowflakeConnection.close()`, after calling
`self._raw_instance.close()`, the `_raw_instance` attribute was **NOT**
reset to `None`:
```python
def close(self) -> None:
"""Closes the underlying Snowflake connection."""
if self._raw_instance is not None:
self._raw_instance.close()
# BUG: _raw_instance was NOT set to None!
```
This caused the following issue:
1. When `close()` was called (e.g., via `on_release` when a cache entry
is evicted), the underlying connection was closed
2. However, `_raw_instance` still referenced the **closed** connection
object
3. The `_instance` property checks `if self._raw_instance is None` to
decide whether to create a new connection:
```python
@Property
def _instance(self) -> RawConnectionT:
if self._raw_instance is None:
self._raw_instance = self._connect(**self._kwargs)
return self._raw_instance
```
4. Since `_raw_instance` wasn't `None`, subsequent access to `_instance`
returned the **CLOSED** connection
5. Any operations on the closed connection failed with "Connection is
closed"
### When This Bug Manifests
The `on_release` callback (which calls `close()`) is triggered when:
- Cache entries expire due to TTL
- Cache is full and oldest entries are evicted (`max_entries`)
- `st.cache_resource.clear()` is called
- For session-scoped caches: when a session disconnects
For global-scoped connections like `st.connection("snowflake")`, this
typically only happens if:
- `st.cache_resource.clear()` is called explicitly
- TTL is set and expires
- `max_entries` is set and exceeded
### Additional Consideration: Snowpark Sessions
When users call `conn.session()`, they get a Snowpark Session that
internally references `self._instance`. If the underlying connection is
closed:
```python
def session(self) -> Session:
if running_in_sis():
return get_active_session()
return Session.builder.configs({"connection": self._instance}).create()
```
Any Snowpark Sessions created from the connection will also fail because
they hold a reference to the now-closed underlying connection object.
## Fix
The fix is simple: reset `_raw_instance` to `None` after closing the
connection:
```python
def close(self) -> None:
"""Closes the underlying Snowflake connection."""
if self._raw_instance is not None:
self._raw_instance.close()
self._raw_instance = None # Added this line
```
This ensures that after `close()` is called, the next access to
`_instance` will create a new connection instead of returning the closed
one.
## Files Changed
1. **`lib/streamlit/connections/snowflake_connection.py`**
- Fixed `close()` method to reset `_raw_instance = None` after closing
2. **`lib/tests/streamlit/connections/snowflake_connection_test.py`**
- Added `TestSnowflakeConnectionClose` test class with:
- `test_close_resets_raw_instance`: Verifies that `close()` closes the
connection AND resets `_raw_instance`
- `test_close_is_noop_when_not_connected`: Verifies that `close()`
doesn't fail when `_raw_instance` is already `None`
## Testing
```bash
PYTHONPATH=lib pytest lib/tests/streamlit/connections/snowflake_connection_test.py::TestSnowflakeConnectionClose -v
```
Output:
```
lib/tests/streamlit/connections/snowflake_connection_test.py::TestSnowflakeConnectionClose::test_close_resets_raw_instance PASSED
lib/tests/streamlit/connections/snowflake_connection_test.py::TestSnowflakeConnectionClose::test_close_is_noop_when_not_connected PASSED
```
## Recommendations for Users
Until this fix is released, users experiencing this issue can:
1. **Avoid storing Snowpark Sessions long-term**: Instead of caching
Snowpark Sessions, create them fresh when needed
2. **Check if using `st.cache_resource.clear()`**: If calling this
anywhere in the app, it will close all cached connections
3. **Consider connection TTL settings**: If TTL is set on the
connection, it may expire and close
## Impact
- **Affected**: Users of `st.connection("snowflake")` and
`st.connection("snowflake-callers-rights")` who experience cache
eviction scenarios
- **Severity**: Medium - The bug causes operations to fail with a
confusing error message, but the workaround (restarting the app or
avoiding cache clears) is available
- **Scope**: Only affects `SnowflakeConnection` and its subclasses;
other connection types (`SQLConnection`, `SnowparkConnection`) inherit
the no-op `close()` from `BaseConnection` and are not affected
</details>
## GitHub Issue Link (if applicable)
## Testing Plan
- Added unit test.
---
**Contribution License Agreement**
By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
Describe your changes
Fixes an issue with Snowflake connections not getting re-initialized after having been closed.
Claude issue analysis
# Issue Report: Snowflake Connection "Connection is closed" ErrorSummary
After recent PRs adding
on_releasetost.cache_resourceand session-scoped connection support, users may encounter asnowflake.connector.errors.DatabaseError: 250002 (08003): Connection is closederror when usingst.connection("snowflake")with cached data queries.Related PRs
on_releasetost.cache_resourceSnowflakeCallersRightsConnectionError Details
Root Cause Analysis
Background
The PRs mentioned added important functionality:
on_releasecallback tost.cache_resource, which is called when cache entries are evictedconnection_factory.pyto use thison_releasecallback to callconnection.close()when a connection is evicted from the cacheThis is the relevant code in
connection_factory.py:The Bug
In
BaseSnowflakeConnection.close(), after callingself._raw_instance.close(), the_raw_instanceattribute was NOT reset toNone:This caused the following issue:
When
close()was called (e.g., viaon_releasewhen a cache entry is evicted), the underlying connection was closedHowever,
_raw_instancestill referenced the closed connection objectThe
_instanceproperty checksif self._raw_instance is Noneto decide whether to create a new connection:Since
_raw_instancewasn'tNone, subsequent access to_instancereturned the CLOSED connectionAny operations on the closed connection failed with "Connection is closed"
When This Bug Manifests
The
on_releasecallback (which callsclose()) is triggered when:max_entries)st.cache_resource.clear()is calledFor global-scoped connections like
st.connection("snowflake"), this typically only happens if:st.cache_resource.clear()is called explicitlymax_entriesis set and exceededAdditional Consideration: Snowpark Sessions
When users call
conn.session(), they get a Snowpark Session that internally referencesself._instance. If the underlying connection is closed:Any Snowpark Sessions created from the connection will also fail because they hold a reference to the now-closed underlying connection object.
Fix
The fix is simple: reset
_raw_instancetoNoneafter closing the connection:This ensures that after
close()is called, the next access to_instancewill create a new connection instead of returning the closed one.Files Changed
lib/streamlit/connections/snowflake_connection.pyclose()method to reset_raw_instance = Noneafter closinglib/tests/streamlit/connections/snowflake_connection_test.pyTestSnowflakeConnectionClosetest class with:test_close_resets_raw_instance: Verifies thatclose()closes the connection AND resets_raw_instancetest_close_is_noop_when_not_connected: Verifies thatclose()doesn't fail when_raw_instanceis alreadyNoneTesting
Output:
Recommendations for Users
Until this fix is released, users experiencing this issue can:
st.cache_resource.clear(): If calling this anywhere in the app, it will close all cached connectionsImpact
st.connection("snowflake")andst.connection("snowflake-callers-rights")who experience cache eviction scenariosSnowflakeConnectionand its subclasses; other connection types (SQLConnection,SnowparkConnection) inherit the no-opclose()fromBaseConnectionand are not affectedGitHub Issue Link (if applicable)
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.