Conversation
…y_repo_id Fixes #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>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| """.format(repo_id, new_endpoint, new_field) | ||
| try: | ||
| with engine.connect() as conn: | ||
| with engine.begin() as conn: |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
E0602: Undefined variable 'engine' (undefined-variable)
| """.format(repo_id, new_endpoint, new_field) | ||
| try: | ||
| with engine.connect() as conn: | ||
| with engine.begin() as conn: |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
E0602: Undefined variable 'engine' (undefined-variable)
| """.format(record['repo_id'], record['ri_metric'], record['ri_field']) | ||
| try: | ||
| with engine.connect() as conn: | ||
| with engine.begin() as conn: |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
E0602: Undefined variable 'engine' (undefined-variable)
| """.format(insight['repo_id'], insight['ri_metric']) | ||
| try: | ||
| with engine.connect() as conn: | ||
| with engine.begin() as conn: |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
E0602: Undefined variable 'engine' (undefined-variable)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This PR was ported from augurlabs/augur#3716 filed by @devs6186 because the contribution was deemed to still be useful.
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