✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

✅ PR preview is ready!

Summary

This PR adds session-scoped connection support to Streamlit's BaseConnection class as part of the approved spec for session-scoped connections and Snowflake RCR (Restricted Caller's Rights). The changes introduce:

1. A new scope() classmethod on BaseConnection that returns "global" (default) or "session" to control how connections are cached
2. A new close() method on BaseConnection that serves as a cleanup hook when connections are released from the cache
3. Updates to connection_factory to pass these values to the underlying cache_resource decorator
4. Comprehensive unit tests covering the new functionality

Code Quality

The code follows Streamlit's established patterns and conventions well:

lib/streamlit/connections/base_connection.py:

• ✅ Proper use of from __future__ import annotations
• ✅ Type annotations using Literal["global", "session"]
• ✅ NumPy-style docstrings explaining the methods' purposes
• ✅ Default implementations maintain backwards compatibility (scope() returns "global", close() is a no-op)

lib/streamlit/runtime/connection_factory.py:

• ✅ Good validation of scope values with a user-friendly error message (lines 100-104)
• ✅ Clean wrapper function for on_release (lines 106-107)
• ✅ Docstring updated to clarify that only "global" scoped connections are shared between sessions (line 236-237)

Design Note: The spec document shows scope as an instance property, but the implementation uses a classmethod. The classmethod approach is actually better design because scope is a characteristic of the connection class, not individual instances. This allows the factory to query the scope before creating instances.

Test Coverage

The test coverage in lib/tests/streamlit/runtime/connection_factory_test.py is comprehensive:

The tests appropriately:

• ✅ Use pytest fixtures and assertions
• ✅ Include docstrings explaining the test purpose
• ✅ Mock cache_resource to verify arguments without testing the caching system itself (which has its own tests)
• ✅ Test error conditions with descriptive error message matching

E2E Tests: No e2e tests were added, which is appropriate. This PR adds infrastructure for future features (session-scoped connections) and the actual functionality is a pass-through to existing cache_resource behavior that already has test coverage.

Backwards Compatibility

Excellent backward compatibility:

1. Default scope() returns "global" - All existing BaseConnection subclasses will continue to work exactly as before with global caching
2. Default close() is a no-op - Existing connections won't break; they simply won't perform cleanup
3. No changes to st.connection() signature - User code calling st.connection() is unaffected
4. Existing first-party connections unchanged - SQLConnection, SnowflakeConnection, and SnowparkConnection don't override these methods, maintaining their current behavior

Security & Risk

Low risk assessment:

• ✅ The changes are additive and don't modify existing behavior paths
• ✅ Scope validation prevents injection of invalid scope values
• ✅ The close() method follows the established pattern from cache_resource's on_release parameter
• ✅ Exception handling in close() is handled by the caching layer (exceptions bubble up appropriately according to existing cache code)

Minor consideration: The close() method could throw exceptions during cleanup. However, reviewing the caching code shows this is already handled - exceptions in on_release functions bubble up as user script errors, and the cache continues clearing other entries. This is consistent with established behavior.

Recommendations

The PR is well-implemented and ready for merge. A few minor optional improvements:

1. Documentation clarity (optional): The updated docstring in connection_factory mentions that "global" scoped connections are shared between sessions, but doesn't explain what happens with "session" scoped connections. Consider adding: "Connection types with a scope of "session" will be cached per-session and not shared."

2. Future consideration (not blocking): When implementing actual session-scoped connections (like the planned Snowflake RCR connection), ensure the close() implementations are thread-safe since they may be called during session teardown from different threads.

Verdict

APPROVED: This PR adds well-designed, backwards-compatible infrastructure for session-scoped connections with comprehensive unit test coverage. The implementation follows Streamlit conventions, aligns with the approved product spec, and introduces no regressions to existing functionality.

This is an automated AI review. Please verify the feedback and use your judgment.