[tasks/github] Fix uncommitted UPDATE in update_issue_closed_cntrbs_by_repo_id#3716
[tasks/github] Fix uncommitted UPDATE in update_issue_closed_cntrbs_by_repo_id#3716devs6186 wants to merge 2 commits intoaugurlabs:mainfrom
Conversation
|
Thanks for the contribution! Can you clarify how this PR relates to the two issues you linked? |
|
|
||
| if update_data: | ||
| with engine.connect() as connection: | ||
| # engine.begin() auto-commits (or rolls back on exception). |
There was a problem hiding this comment.
Holy moly. This comment is right. Nice catch.
There was a problem hiding this comment.
Holy moly. This comment is right. Nice catch.
Thanks! Took me a while to trace why the field was always null — the engine.connect() vs engine.begin()
distinction in 2.0 is easy to miss.
There was a problem hiding this comment.
this comment also tracks with a fix that caio identified in 8Knot.
Sure. The traceback in #3457 shows the crash path going through _process_events to update_issue_closed_cntrbs_by_repo_id. The root cause is that engine.connect() in SQLAlchemy 2.0 uses autobegin but not autocommit — so the UPDATE on cntrb_id was being silently rolled back on every single call, leaving the field perpetually stale. The try/except wrapper means a failure there no longer kills the entire event collection batch for that repo. The link to #3192 is more indirect — the original reporter of #3457 noted the bug pattern looks similar (data |
MoralCode
added
the
stale
|
Taking over this PR since the contributor has abandoned it and the changes it makes are fairly minimal and seemingly salvage-able |
…y_repo_id Fixes augurlabs#3457 - Replace engine.connect() with engine.begin() so the UPDATE on issues.cntrb_id is actually committed; previously every call was silently rolled back at context-manager exit in SQLAlchemy 2.0 - Wrap the call site in events.py with try/except so a transient DB error here does not abort the entire batch of event collection Signed-off-by: devs6186 <devyanshsomvanshi@gmail.com> Signed-off-by: Adrian Edwards <adredwar@redhat.com>
…tually perform updates to data Signed-off-by: Adrian Edwards <adredwar@redhat.com>
MoralCode
force-pushed
the
fix/3457-events-update-transaction
branch
from
0fa2f49 to
b9eb8de
Compare
Changeset
Notes
In SQLAlchemy 2.0 engine.connect() uses autobegin but NOT autocommit. Every call to update_issue_closed_cntrbs_by_repo_id was rolling back at context-manager exit, leaving cntrb_id stale. Switching to engine.begin() makes the transaction commit on success and roll back only on exception, which is the correct behavior. The try/except guard in _process_events means a transient failure here logs an error rather than killing the Celery task mid-batch.
Related issues/PRs
Description
Notes for Reviewers
The engine.connect() -> engine.begin() change is the core fix. Confirm the UPDATE path in lib.py and the error-handling wrapper in events.py look correct.
Signed commits