Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PP-2055] Use Ensure that local analytics events are committed to the… #2250

Merged

Conversation

dbernstein
Copy link
Contributor

@dbernstein dbernstein commented Jan 14, 2025

… database by running the event collection within a transaction.

Also use one transaction per event collected to minimize database lock durations.

Before this update, we were using task.session() which does not automatically commit when used in a with block. While the S3 analytics provider doesn't make any database updates, the Local Analytics Provider does. As a result, the updates to the local analytics provider which is backed by the database were not being committed.

Motivation and Context

https://ebce-lyrasis.atlassian.net/browse/PP-2055

How Has This Been Tested?

Checklist

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

… database by running the event collection within a transaction.

Also use one transaction per event collected to minimize database lock durations.
@dbernstein dbernstein marked this pull request as draft January 14, 2025 22:31
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.11%. Comparing base (f417b02) to head (f23c89e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2250   +/-   ##
=======================================
  Coverage   91.11%   91.11%           
=======================================
  Files         363      363           
  Lines       41296    41297    +1     
  Branches     8840     8841    +1     
=======================================
+ Hits        37628    37629    +1     
  Misses       2406     2406           
  Partials     1262     1262           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dbernstein dbernstein marked this pull request as ready for review January 14, 2025 23:56
@dbernstein dbernstein merged commit c362024 into main Jan 15, 2025
21 checks passed
@dbernstein dbernstein deleted the PP-2055-ensure-local-analytics-are-included-in-celery-tasks branch January 15, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants