Add engine accessor to SQLAlchemySession for closing it when it's created from a URL

[gn00295120]

Problem

Interface Inconsistency

Currently, SQLAlchemySession lacks a close() method, making it inconsistent with other session implementations:

• ✅ RedisSession has async def close()
• ✅ SQLiteSession has def close()
• ❌ SQLAlchemySession has no close() method

This inconsistency means users cannot properly clean up resources when using from_url().

Resource Leak in Long-Running Applications

When using from_url(), each call creates a new AsyncEngine with its own connection pool:

# This pattern leaks engines in long-running apps
@app.post("/chat/{user_id}")
async def chat(user_id: str):
    session = SQLAlchemySession.from_url(
        session_id=f"user_{user_id}",
        url=DATABASE_URL
    )
    # ... use session ...
    # ❌ No way to clean up the engine!

SQLAlchemy Official Requirement

From SQLAlchemy Discussion #7531:

"invoke the AsyncEngine.dispose() method using await when the AsyncEngine object will go out of scope... Failing to explicitly dispose may result in warnings like 'RuntimeError: Event loop is closed' within garbage collection."

---

Why This Change Is Needed

1. API Consistency

Users expect all session types to have consistent lifecycle management. The close() method should be available across all session implementations.

2. Follows Best Practices

SQLAlchemy's async engine requires explicit disposal. Without it, applications may see:

• Connection pool exhaustion
• "Event loop is closed" warnings during GC
• Resource leaks in long-running services

3. Prevents Resource Leaks

In production web services handling multiple users, each from_url() call accumulates engines that are never cleaned up.

4. Backward Compatible

This is a non-breaking change:

• Adds optional close() method
• Existing code continues to work
• Only affects users who explicitly call close()

---

Solution

Following the same pattern as RedisSession:

1. Track Engine Ownership

self._owns_engine = False  # Default: external engine

2. Mark Ownership in from_url()

session = cls(session_id, engine=engine, **kwargs)
session._owns_engine = True  # SDK created this engine
return session

3. Add close() Method

async def close(self) -> None:
    """Dispose engine only if we own it."""
    if self._owns_engine:
        await self._engine.dispose()

---

Usage Examples

Example 1: Using from_url() (SDK owns engine)

session = SQLAlchemySession.from_url(
    "user_123",
    url="postgresql+asyncpg://localhost/db"
)
try:
    await session.add_items([...])
finally:
    await session.close()  # ✅ Engine disposed

Example 2: External engine (user owns engine)

engine = create_async_engine("postgresql+asyncpg://localhost/db")
session = SQLAlchemySession("user_123", engine=engine)

await session.add_items([...])
await session.close()  # ✅ Engine NOT disposed (user manages it)

# User responsible for cleanup
await engine.dispose()

---

Testing

Tested manually with both scenarios:

• ✅ from_url() disposes engine on close()
• ✅ External engine is NOT disposed on close()
• ✅ Multiple sessions clean up properly

---

Changes

• Add _owns_engine flag to track engine ownership
• from_url() sets _owns_engine=True
• Direct engine injection keeps _owns_engine=False
• New async def close() method
• Matches RedisSession resource management pattern

---

Impact

Scope: Long-running applications using from_url() in multi-user scenarios

Severity: Medium - Not critical for short-lived scripts, but important for production web services

Compatibility: Fully backward compatible - existing code unaffected

[Copilot]

Pull Request Overview

This PR adds a close() method to SQLAlchemySession to enable proper engine disposal and prevent resource leaks in long-running applications. The implementation follows the same ownership pattern as RedisSession.

Key Changes:

• Adds _owns_engine flag to track whether the session owns its engine
• Sets _owns_engine=True when using from_url() factory method
• Implements async def close() method that disposes the engine only when owned

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[gn00295120]

Thanks for the review! I've updated the docstring to include the Returns: None section as suggested, ensuring consistency with Python documentation conventions.

[seratch]

I think simply adding the accessor to _engine would be simpler and more flexible

[gn00295120]

Thank you for the feedback! I've updated the PR to add the engine property accessor while keeping the existing close() method.

Now users have both options:

• Convenience: await session.close() - automatically handles engine disposal based on ownership
• Flexibility: session.engine - direct access for advanced use cases (checking pool status, manual disposal, etc.)

This approach:

• Provides the simplicity and flexibility you suggested via the engine accessor
• Maintains consistency with RedisSession and SQLiteSession which both have close() methods
• Keeps the ownership tracking to prevent accidentally disposing shared engines

Does this align with what you had in mind? Happy to adjust further if you prefer a different approach.

[seratch]

I'd like to remove this general method because the sessions store interface does not declare this method

[seratch]

No, I don't think we should add close() method

[gn00295120]

Thank you for the clarification! I completely understand now.

I've updated the PR to follow your suggestion:

• ✅ Added engine property accessor for direct access to AsyncEngine
• ❌ Removed close() method (not part of SessionABC interface)
• ❌ Removed ownership tracking (_owns_engine)

Now the implementation is simple and clean - just a property accessor. Users are responsible for managing the engine lifecycle themselves using await session.engine.dispose() when needed.

This aligns with the SessionABC interface and keeps the implementation minimal.

[gn00295120]

I've added comprehensive unit tests to verify the engine property behavior:

Test Coverage

1. test_engine_property_from_url()

   • Verifies engine property returns AsyncEngine instance
   • Tests that users can access pool status
   • Confirms manual disposal works: await session.engine.dispose()

2. test_engine_property_from_external_engine()

   • Verifies engine property returns the same external engine instance
   • Ensures users retain control when injecting their own engine

3. test_engine_property_is_read_only()

   • Confirms property is immutable (raises AttributeError on assignment)
   • Ensures engine reference cannot be accidentally modified

✅ Local Test Results

All tests pass successfully:

tests/extensions/memory/test_sqlalchemy_session.py::test_engine_property_from_url PASSED
tests/extensions/memory/test_sqlalchemy_session.py::test_engine_property_from_external_engine PASSED
tests/extensions/memory/test_sqlalchemy_session.py::test_engine_property_is_read_only PASSED

============================== 3 passed in 0.14s ===============================

The tests follow the existing test patterns in the file and use the same in-memory SQLite setup.

[gn00295120]

Fixed CI failures:

1. mypy error - Added # type: ignore[misc] to the intentional read-only property assignment test
2. lint errors - Shortened docstrings to stay within 100 character limit

All tests still pass locally:

tests/extensions/memory/test_sqlalchemy_session.py::test_engine_property_from_url PASSED
tests/extensions/memory/test_sqlalchemy_session.py::test_engine_property_from_external_engine PASSED  
tests/extensions/memory/test_sqlalchemy_session.py::test_engine_property_is_read_only PASSED

============================== 3 passed in 0.14s ===============================

CI should pass now.