Summary
While looking through delete_gapanalysis_results_for() in application/database/db.py, I noticed that Session.delete() is being used as if it returns a result object.
However, in SQLAlchemy ORM, session.delete() returns None, so the current logging logic doesn’t behave as intended.
Current behavior
The method does something like:
result = self.session.delete(r)
if result: logger.info(...)
Since delete() returns None, this condition never triggers, and the logging branch is effectively dead.
Expected behavior
- Rows should still be deleted correctly
- Logging should reflect actual deletions using a reliable value (e.g. number of rows processed)
Why this matters
This can be confusing for contributors reading the code, since it suggests delete() returns metadata like rowcount, which isn’t the case.
Cleaning this up would make the code easier to understand and avoid misleading patterns.
Suggested approach
- Remove reliance on the return value of
session.delete()
- Optionally log based on the number of matched rows (e.g. length of query result or bulk delete count)
Scope
Small, localized fix in:
application/database/db.py → delete_gapanalysis_results_for()
I’ll go ahead and open a PR for this—happy to adjust it based on any feedback.
Summary
While looking through
delete_gapanalysis_results_for()inapplication/database/db.py, I noticed thatSession.delete()is being used as if it returns a result object.However, in SQLAlchemy ORM,
session.delete()returnsNone, so the current logging logic doesn’t behave as intended.Current behavior
The method does something like:
result = self.session.delete(r)if result: logger.info(...)Since
delete()returnsNone, this condition never triggers, and the logging branch is effectively dead.Expected behavior
Why this matters
This can be confusing for contributors reading the code, since it suggests
delete()returns metadata likerowcount, which isn’t the case.Cleaning this up would make the code easier to understand and avoid misleading patterns.
Suggested approach
session.delete()Scope
Small, localized fix in:
application/database/db.py→delete_gapanalysis_results_for()I’ll go ahead and open a PR for this—happy to adjust it based on any feedback.