asf-tooling commented on issue #299:
URL:
https://github.com/apache/tooling-trusted-releases/issues/299#issuecomment-4404074221
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@837830e8`
**Type:** `discussion` • **Classification:** `no_action` •
**Confidence:** `high`
**Application domain(s):** `data_storage`
### Summary
This is an ongoing architectural design discussion about preventing use of
detached SQLAlchemy/SQLModel session objects after their session closes. The
team has identified three distinct failure modes (detached-writes,
detached-reads, and greenlet-lazy-load) but has not converged on a single
implementation approach. @sbp listed five potential solutions ranging from
one-session-per-request to continuation passing style (CPS), and most recently
(Apr 23 2026) noted the need to verify that live session data isn't returned
from continuation functions. @dave2wave characterized the near-term approach as
'awareness while developing.' The CPS approach is being tested first in #628.
### Where this lives in the code today
#### `atr/db/interaction.py` — `releases_by_phase` (lines 480-501)
_currently does this_
Returns ORM objects that become detached once the session context manager
exits — the core pattern this issue targets.
```python
async def releases_by_phase(project: sql.Project, phase: sql.ReleasePhase)
-> list[sql.Release]:
"""Get the releases for the project by phase."""
query = (
sqlmodel.select(sql.Release)
.where(
sql.Release.project_key == project.key,
sql.Release.phase == phase,
)
.order_by(sql.validate_instrumented_attribute(sql.Release.created).desc())
)
results = []
async with db.session() as data:
for result in (await data.execute(query)).all():
release = result[0]
results.append(release)
for release in results:
# Don't need to eager load and lose it when the session closes
release.project = project
return results
```
#### `atr/db/interaction.py` — `all_releases` (lines 107-118)
_currently does this_
Another instance of the pattern: opens a session, fetches ORM objects,
closes the session, then manually assigns a relationship. The returned objects
are detached.
```python
async def all_releases(project: sql.Project) -> list[sql.Release]:
"""Get all releases for the project, sorted by version."""
query = sqlmodel.select(sql.Release).where(sql.Release.project_key ==
project.key)
results = []
async with db.session() as data:
for result in (await data.execute(query)).all():
release = result[0]
results.append(release)
for release in results:
release.project = project
```
#### `atr/storage/__init__.py` — `write` (lines 455-463)
_needs modification_
The session is scoped to this context manager, but Write (and its writer
subsystems) may return ORM objects that outlive the session. @sbp's latest
comment addresses this: 'check that live session data is not returned from any
of the continuation functions.'
```python
@contextlib.asynccontextmanager
async def write(asf_uid: principal.UID = principal.ArgumentNone) ->
AsyncGenerator[Write]:
if asf_uid is principal.ArgumentNone:
authorisation = await principal.Authorisation()
else:
authorisation = await principal.Authorisation(asf_uid)
async with db.session() as data:
# TODO: Replace data with a DatabaseWriter instance
yield Write(authorisation, data)
```
#### `atr/storage/__init__.py` — `read` (lines 455-463)
_needs modification_
Same session-scoping pattern as write(); ORM objects retrieved via Read can
escape the session scope.
```python
@contextlib.asynccontextmanager
async def read(asf_uid: principal.UID = principal.ArgumentNone) ->
AsyncGenerator[Read]:
if asf_uid is principal.ArgumentNone:
authorisation = await principal.Authorisation()
else:
authorisation = await principal.Authorisation(asf_uid)
async with db.session() as data:
# TODO: Replace data with a DatabaseReader instance
yield Read(authorisation, data)
```
#### `atr/db/interaction.py` — `release_ready_for_vote` (lines 436-451)
_currently does this_
The specific function @andrewmusselman's analysis cited — it loads
with_project_release_policy but not with_release_policy, causing
MissingGreenlet on lazy access (failure mode C).
```python
async def release_ready_for_vote(
session: web.Committer,
project_key: safe.ProjectKey,
version_key: safe.VersionKey,
revision: safe.RevisionNumber,
data: db.Session,
allowed_vote_modes: frozenset[sql.VoteMode],
) -> tuple[sql.Release, sql.Committee] | str:
release = await session.release(
project_key,
version_key,
data=data,
with_project=True,
with_committee=True,
with_project_release_policy=True,
)
```
#### `atr/storage/writers/cache.py` — `GeneralPublic` (lines 30-40)
_currently does this_
Writer class that stores a db.Session reference — if the session closes
before the writer is used, operations on stale objects may fail silently or
with detached errors.
```python
class GeneralPublic:
def __init__(
self,
write: storage.Write,
write_as: storage.WriteAsGeneralPublic,
data: db.Session,
) -> None:
self.__write = write
self.__write_as = write_as
self.__data = data
self.__asf_uid = write.authorisation.asf_uid
```
### Proposed approach
The team has not converged on a single implementation approach. @sbp
identified continuation passing style (CPS) as the most complete solution — it
prevents detached objects from leaking out of session scopes by construction —
but acknowledged it's cumbersome. This is being tested first in issue #628
(quarantined mode). @dave2wave characterized the near-term approach as
developer awareness. @sbp's most recent comment (Apr 23) suggests the next
concrete step is checking that live session data is not returned from
continuation functions. Until #628 yields results and the team decides on an
approach, no unilateral patch should be proposed.
### Open questions
- What was the outcome of the CPS experiment in #628? Has it been
implemented or is it still in progress?
- Does @dave2wave's question about per-connection pragmas refer to SQLite
PRAGMA statements that could help with session isolation?
- Would a runtime check (e.g., asserting objects are not detached before
passing them across boundaries) be acceptable as an interim measure?
- Should there be a linting rule or custom mypy/pyright plugin to flag ORM
objects escaping session scopes?
- Are there specific writer methods that have been identified as currently
returning detached ORM objects that callers rely on?
_The agent reviewed this issue and is not proposing patches in this run.
Review the existing-code citations and open questions above before deciding
next steps._
### Files examined
- `atr/db/__init__.py`
- `atr/storage/writers/cache.py`
- `atr/db/interaction.py`
- `atr/storage/writers/mail.py`
- `atr/storage/__init__.py`
---
*Draft from a triage agent. A human reviewer should validate before merging
any change. The agent did not run tests or verify diffs apply.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
