-
Notifications
You must be signed in to change notification settings - Fork 80
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
LJ-453 #5834
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, we should make sure we ticket the quirks around TaskResources
and GraphTask
session handling
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (95.23%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #5834 +/- ##
==========================================
+ Coverage 86.62% 86.96% +0.33%
==========================================
Files 406 407 +1
Lines 25017 25038 +21
Branches 2695 2696 +1
==========================================
+ Hits 21672 21775 +103
+ Misses 2771 2680 -91
- Partials 574 583 +9 ☔ View full report in Codecov by Sentry. |
fides
|
Project |
fides
|
Branch Review |
main
|
Run status |
|
Run duration | 00m 50s |
Commit |
|
Committer | erosselli |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
5
|
View all changes introduced in this branch ↗︎ |
Closes LJ-453
Description Of Changes
Because some privacy requests can take very long to process (e.g 40min) , by the time some access tasks complete, the database session they were using has been closed and so we get SSL / transaction errors when they try to update their own status. Since they throw before they can save their status in the DB, these tasks get re-processed indefinitely when re-submitting the request.
This PR fixes this by using a brand new DB session every time
GrpahTask.update_status
gets called.Code Changes
get_messaging_service
,get_privacy_request_service
,get_dataset_service
andget_dataset_config_service
fromsrc/fides/api/api/deps.py
into their own filesrc/fides/api/service/deps.py
to avoid circular importsGraphTask
methodsupdate_status
andlog_end
to use new DB sessionsSteps to Confirm
Pre-Merge Checklist
CHANGELOG.md
updatedmain
downgrade()
migration is correct and works