Adds celery==5.2.7

[seanpreston]

Purpose

This PR adds the Celery dependency back into the project.

Changes

• Adds celery and the [pytest] dependencies
• Adds some basic broker config. for Celery
• Adds locations to register tasks from (though this is not limiting)
• Adds a virtual celery worker to all of our tests, forcing any tasks invoked during a test into the virtualised execution environment — this saves us needing to run any extra worker nodes during the tests
• Adds a test to check the task dispatch

Please note

• The increment updating the execution layer to use Celery for privacy request execution will follow in this separate PR.

Checklist

• Update CHANGELOG.md file
  • Merge in main so the most recent CHANGELOG.md file is being appended to
  • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
  • Add a link to this PR at the end of the description with the PR number as the text. example: #1
• Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
• If docs updated (select one):
  • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
  • documentation issue created (tag docs-team to complete issue separately)
• Good unit test/integration test coverage
• This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
• The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #524

[seanpreston]

Hey @eastandwestwind, as far as I'm aware we don't want to commit this correct?

[eastandwestwind]

correct, we don't want this committed. I'll update such that we don't generate it if we have the ENV var set in this ticket- #559

[eastandwestwind]

hey @seanpreston just checking, do you have the env var FIDESOPS__ROOT_USER__ANALYTICS_ID set to internal? We should already skip writing the analytics_id for internal users, but lemme know if if still generates the id after you set the var.

[seanpreston]

Looks like those integration tests are also failing locally and remotely on main:

FAILED tests/integration_tests/saas/test_segment_task.py::test_segment_saas_access_request_task - AssertionError
FAILED tests/integration_tests/saas/test_sentry_task.py::test_sentry_access_request_task - AssertionError
FAILED tests/integration_tests/saas/test_sentry_task.py::test_sentry_erasure_request_task - Exception: The issues endpoint d...
FAILED tests/integration_tests/saas/test_stripe_task.py::test_stripe_access_request_task - AssertionError

[pattisdr]

looks good Sean

[pattisdr]

This changelog is looking out of date?

[seanpreston]

Anything specific you can see that's missing here? Merging in main to this branch isn't yielding any conflicts or extra lines.

[pattisdr]

I just thought 1.5.3 had already been released -

[pattisdr]

Chatted in person, my mistake, 1.5.3 was released from a different branch

[pattisdr]

I missed this, is this other toml file just used for tests? Can we add a code comment about what this file is for?

[seanpreston]

I was also surprised to find this. Yes this file is just used in one test to check we can load the config in from a different path. The catch is that because of the way we validate configs, we need all the required vars in here (lest it throws a ValidationError as it should). Hence it looking the same as our actual config file.

Will add a comment 👍

[pattisdr]

This reminds me we were using celery back in the day and switched to apscheduler for some onetrust things (we also use it for webhook cleanup tasks too). Nothing to change here, just noting we might revert back to celery one day here to consolidate dependencies.

[seanpreston]

I thought about this too, though it seemed like something we can save for another increment.

There's also a serious discussion for us to have around ripping the Onetrust integration out entirely, since it's not used. This came up while removing PrivacyRequestRunner.submit() calls in the next PR, since we can't effectively test the Onetrust integration.

[pattisdr]

Note to self, this uses Celery's pytest fixtures https://docs.celeryq.dev/en/stable/userguide/testing.html#function-scope

[seanpreston]

There's a testing issue I'm having in the next PR where celery_app the Pytest fixture — which will automatically use the virtualised celery_worker instead of looking for the one via the broker (Redis, which isn't created as part of the test suite) — isn't being automatically used to process tasks invoked in tests.

That doesn't show up here since the new task is added to celery_app directly. I've circumvented it in that in the next PR by addressing the task directly from celery_app.tasks[...], then calling that directly.