Add dedicated Celery queue for bulk consent imports#7743
Add dedicated Celery queue for bulk consent imports#7743
Conversation
Add a dedicated Celery queue (fidesplus.bulk_consent_import) so that bulk pre-verified consent record imports do not compete with the main privacy_preferences queue used by normal consent operations.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Greptile SummaryThis PR introduces a dedicated Celery queue (
Confidence Score: 5/5
Important Files Changed
Reviews (1): Last reviewed commit: "Add BULK_CONSENT_IMPORT_QUEUE_NAME for b..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review Summary
The change itself is correct and follows the established pattern exactly — constant naming, queue string format, inline comment style, alphabetical import ordering, and placement in all_queues all match existing conventions. The motivation (isolating bulk imports from the live consent queue) is sound.
Critical (Must Fix)
tests/ctl/api/test_worker.py is not updated. The test_start_worker_with_arguments parametrized test has hardcoded expected queue strings that omit fidesplus.bulk_consent_import. Three of the five parametrized cases will fail:
- The "all queues" case (
queues=None, exclude_queues=None) — the actual result will include the new queue at the end. - The "exclude some queues" case where
fidesplus.bulk_consent_importis not in the excluded set. - The "exclude fides,fides.dsr" case — the new queue survives and appears in the result.
The fix is to append fidesplus.bulk_consent_import to the expected queue strings in each affected test case. See the inline comment for the exact corrected string for the all-queues case.
Everything Else
No other issues — the two-file change is clean, minimal, and consistent with the codebase pattern.
vcruces
left a comment
There was a problem hiding this comment.
This looks good to me! Quick question, would it be useful to include the queue in get_queue_counts (src/fides/api/util/cache.py), or is it not worth it?
|
Done — updated the three affected parametrized test cases in |
|
Good call — added it to |
- Add fidesplus.bulk_consent_import to expected queue strings in test_start_worker_with_arguments - Add BULK_CONSENT_IMPORT_QUEUE_NAME to get_queue_counts for health/workers endpoint visibility
Summary
BULK_CONSENT_IMPORT_QUEUE_NAME(fidesplus.bulk_consent_import) constant tofides.api.tasksall_queueslist so it is consumed by defaultThis queue isolates bulk pre-verified consent record imports from the main
fides.privacy_preferencesqueue, preventing large imports (e.g. 4M historical records) from blocking normal consent operations.Test plan