Skip to content

Commit

Permalink
🚚(celery) move and rename task files and task functions
Browse files Browse the repository at this point in the history
Relocating and renaming tasks files `deletion_tasks` and `emailing_tasks` to
`tasks.deletion` and `tasks.emailing`.
Renaming `delete_user` utility function to `delete_user_from_db`.
Renaming `deletion_task` task function to `delete_user`.
Renaming `send_email` task function to `warn_user`.
  • Loading branch information
wilbrdt committed Oct 16, 2024
1 parent d09ab39 commit 2f7c549
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 96 deletions.
12 changes: 6 additions & 6 deletions src/app/mork/api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

from pydantic import BaseModel

from mork.celery.deletion_tasks import delete_inactive_users
from mork.celery.emailing_tasks import warn_inactive_users
from mork.celery.tasks.deletion import delete_inactive_users
from mork.celery.tasks.emailing import warn_inactive_users


@unique
Expand All @@ -25,8 +25,8 @@ class TaskStatus(str, Enum):
class TaskType(str, Enum):
"""Possible task types."""

EMAILING = "emailing"
DELETION = "deletion"
EMAIL_INACTIVE_USERS = "email_inactive_users"
DELETE_INACTIVE_USERS = "delete_inactive_users"


class TaskCreate(BaseModel):
Expand All @@ -43,6 +43,6 @@ class TaskResponse(BaseModel):


TASK_TYPE_TO_FUNC = {
TaskType.EMAILING: warn_inactive_users,
TaskType.DELETION: delete_inactive_users,
TaskType.EMAIL_INACTIVE_USERS: warn_inactive_users,
TaskType.DELETE_INACTIVE_USERS: delete_inactive_users,
}
2 changes: 1 addition & 1 deletion src/app/mork/celery/celery_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from celery import Celery

app = Celery(
"mork", include=["mork.celery.deletion_tasks", "mork.celery.emailing_tasks"]
"mork", include=["mork.celery.tasks.deletion", "mork.celery.tasks.emailing"]
)

# Using a string here means the worker doesn't have to serialize
Expand Down
1 change: 1 addition & 0 deletions src/app/mork/celery/tasks/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# noqa: D104
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,18 @@ def delete_inactive_users():
offset=batch_offset,
limit=settings.EDX_QUERY_BATCH_SIZE,
)
delete_group = group([deletion_task.s(user.email) for user in inactive_users])
delete_group = group([delete_user.s(user.email) for user in inactive_users])
delete_group.delay()


@app.task(
bind=True,
retry_kwargs={"max_retries": settings.DELETE_MAX_RETRIES},
)
def deletion_task(self, email: str):
def delete_user(self, email: str):
"""Celery task that delete a specified user."""
try:
delete_user(email)
delete_user_from_db(email)
except UserDeleteError as exc:
logger.exception(exc)
raise self.retry(exc=exc) from exc
Expand All @@ -51,7 +51,7 @@ def deletion_task(self, email: str):
delete_email_status(email)


def delete_user(email):
def delete_user_from_db(email):
"""Delete user from edX database."""
db = OpenEdxDB()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def warn_inactive_users():
limit=settings.EDX_QUERY_BATCH_SIZE,
)
send_email_group = group(
[send_email_task.s(user.email, user.username) for user in inactive_users]
[warn_user.s(user.email, user.username) for user in inactive_users]
)
send_email_group.delay()

Expand All @@ -44,8 +44,8 @@ def warn_inactive_users():
rate_limit=settings.EMAIL_RATE_LIMIT,
retry_kwargs={"max_retries": settings.EMAIL_MAX_RETRIES},
)
def send_email_task(self, email_address: str, username: str):
"""Celery task that sends an email to the specified user."""
def warn_user(self, email_address: str, username: str):
"""Celery task that warns the specified user by sending an email."""
# Check that user has not already received a warning email
if check_email_already_sent(email_address):
raise EmailAlreadySent("An email has already been sent to this user")
Expand Down
7 changes: 5 additions & 2 deletions src/app/mork/tests/api/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ async def test_tasks_auth(http_client: AsyncClient):


@pytest.mark.anyio
@pytest.mark.parametrize("task_type", ["emailing", "deletion"])
@pytest.mark.parametrize("task_type", ["email_inactive_users", "delete_inactive_users"])
async def test_create_task(
http_client: AsyncClient, auth_headers: dict, task_type: str
):
Expand Down Expand Up @@ -74,7 +74,10 @@ async def test_get_available_tasks(http_client: AsyncClient, auth_headers: dict)
response_data = response.json()
assert response.status_code == 200
assert response.headers["allow"] == "POST"
assert sorted(response_data.get("task_types")) == ["deletion", "emailing"]
assert sorted(response_data.get("task_types")) == [
"delete_inactive_users",
"email_inactive_users",
]


@pytest.mark.anyio
Expand Down
84 changes: 44 additions & 40 deletions src/app/mork/tests/celery/test_deletion_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
from sqlalchemy import select
from sqlalchemy.exc import SQLAlchemyError

from mork.celery.deletion_tasks import (
from mork.celery.tasks.deletion import (
delete_email_status,
delete_inactive_users,
delete_user,
deletion_task,
delete_user_from_db,
)
from mork.edx import crud
from mork.edx.factories.auth import EdxAuthUserFactory
Expand Down Expand Up @@ -42,19 +42,19 @@ def test_delete_inactive_users(edx_db, monkeypatch):
email="[email protected]",
)

monkeypatch.setattr("mork.celery.deletion_tasks.OpenEdxDB", lambda *args: edx_db)
monkeypatch.setattr("mork.celery.tasks.deletion.OpenEdxDB", lambda *args: edx_db)

mock_group = Mock()
monkeypatch.setattr("mork.celery.deletion_tasks.group", mock_group)
mock_deletion_task = Mock()
monkeypatch.setattr("mork.celery.deletion_tasks.deletion_task", mock_deletion_task)
monkeypatch.setattr("mork.celery.tasks.deletion.group", mock_group)
mock_delete_user = Mock()
monkeypatch.setattr("mork.celery.tasks.deletion.delete_user", mock_delete_user)

delete_inactive_users()

mock_group.assert_called_once_with(
[
mock_deletion_task.s(email="[email protected]"),
mock_deletion_task.s(email="[email protected]"),
mock_delete_user.s(email="[email protected]"),
mock_delete_user.s(email="[email protected]"),
]
)

Expand All @@ -71,70 +71,74 @@ def test_delete_inactive_users_with_batch_size(edx_db, monkeypatch):
email="[email protected]",
)

monkeypatch.setattr("mork.celery.deletion_tasks.OpenEdxDB", lambda *args: edx_db)
monkeypatch.setattr("mork.celery.tasks.deletion.OpenEdxDB", lambda *args: edx_db)

mock_group = Mock()
monkeypatch.setattr("mork.celery.deletion_tasks.group", mock_group)
mock_deletion_task = Mock()
monkeypatch.setattr("mork.celery.deletion_tasks.deletion_task", mock_deletion_task)
monkeypatch.setattr("mork.celery.tasks.deletion.group", mock_group)
mock_delete_user = Mock()
monkeypatch.setattr("mork.celery.tasks.deletion.delete_user", mock_delete_user)

# Set batch size to 1
monkeypatch.setattr("mork.celery.deletion_tasks.settings.EDX_QUERY_BATCH_SIZE", 1)
monkeypatch.setattr("mork.celery.tasks.deletion.settings.EDX_QUERY_BATCH_SIZE", 1)

delete_inactive_users()

mock_group.assert_has_calls(
[
call(
[
mock_deletion_task.s(email="[email protected]"),
mock_delete_user.s(email="[email protected]"),
]
),
call().delay(),
call(
[
mock_deletion_task.s(email="[email protected]"),
mock_delete_user.s(email="[email protected]"),
]
),
call().delay(),
]
)


def test_deletion_task(monkeypatch):
"""Test the `deletion_task` function."""
mock_delete_user = Mock()
monkeypatch.setattr("mork.celery.deletion_tasks.delete_user", mock_delete_user)
def test_delete_user(monkeypatch):
"""Test the `delete_user` function."""
mock_delete_user_from_db = Mock()
monkeypatch.setattr(
"mork.celery.tasks.deletion.delete_user_from_db", mock_delete_user_from_db
)
mock_delete_email_status = Mock()
monkeypatch.setattr(
"mork.celery.deletion_tasks.delete_email_status", mock_delete_email_status
"mork.celery.tasks.deletion.delete_email_status", mock_delete_email_status
)
email = "[email protected]"
deletion_task(email)
delete_user(email)

mock_delete_user.assert_called_once_with(email)
mock_delete_user_from_db.assert_called_once_with(email)
mock_delete_email_status.assert_called_once_with(email)


def test_deletion_task_delete_failure(monkeypatch):
"""Test the `deletion_task` function with a delete failure."""
def test_delete_user_failure(monkeypatch):
"""Test the `delete_user` function with a delete failure."""

def mock_delete(*args):
def mock_delete_user_from_db(*args):
raise UserDeleteError("An error occurred")

monkeypatch.setattr("mork.celery.deletion_tasks.delete_user", mock_delete)
monkeypatch.setattr(
"mork.celery.tasks.deletion.delete_user_from_db", mock_delete_user_from_db
)

with pytest.raises(UserDeleteError, match="An error occurred"):
deletion_task("[email protected]")
delete_user("[email protected]")


def test_delete_user(edx_db, monkeypatch):
"""Test the `delete_user` function."""
def test_delete_user_from_db(edx_db, monkeypatch):
"""Test the `delete_user_from_db` function."""
EdxAuthUserFactory._meta.sqlalchemy_session = edx_db.session
EdxAuthUserFactory.create(email="[email protected]")
EdxAuthUserFactory.create(email="[email protected]")

monkeypatch.setattr("mork.celery.deletion_tasks.OpenEdxDB", lambda *args: edx_db)
monkeypatch.setattr("mork.celery.tasks.deletion.OpenEdxDB", lambda *args: edx_db)

assert crud.get_user(
edx_db.session,
Expand All @@ -145,7 +149,7 @@ def test_delete_user(edx_db, monkeypatch):
email="[email protected]",
)

delete_user(email="[email protected]")
delete_user_from_db(email="[email protected]")

assert not crud.get_user(
edx_db.session,
Expand All @@ -157,19 +161,19 @@ def test_delete_user(edx_db, monkeypatch):
)


def test_delete_user_with_failure(edx_db, monkeypatch):
"""Test the `delete_user` function with a commit failure."""
def test_delete_user_from_db_with_failure(edx_db, monkeypatch):
"""Test the `delete_user_from_db` function with a commit failure."""
EdxAuthUserFactory._meta.sqlalchemy_session = edx_db.session
EdxAuthUserFactory.create(email="[email protected]")

def mock_session_commit():
raise SQLAlchemyError("An error occurred")

edx_db.session.commit = mock_session_commit
monkeypatch.setattr("mork.celery.deletion_tasks.OpenEdxDB", lambda *args: edx_db)
monkeypatch.setattr("mork.celery.tasks.deletion.OpenEdxDB", lambda *args: edx_db)

with pytest.raises(UserDeleteError, match="Failed to delete user."):
delete_user(email="[email protected]")
delete_user_from_db(email="[email protected]")


def test_delete_email_status(db_session, monkeypatch):
Expand All @@ -179,7 +183,7 @@ class MockMorkDB:
session = db_session

EmailStatusFactory._meta.sqlalchemy_session = db_session
monkeypatch.setattr("mork.celery.deletion_tasks.MorkDB", MockMorkDB)
monkeypatch.setattr("mork.celery.tasks.deletion.MorkDB", MockMorkDB)

email = "[email protected]"
EmailStatusFactory.create(email=email)
Expand All @@ -203,7 +207,7 @@ class MockMorkDB:
session = db_session

EmailStatusFactory._meta.sqlalchemy_session = db_session
monkeypatch.setattr("mork.celery.deletion_tasks.MorkDB", MockMorkDB)
monkeypatch.setattr("mork.celery.tasks.deletion.MorkDB", MockMorkDB)

email = "[email protected]"

Expand All @@ -212,7 +216,7 @@ class MockMorkDB:
delete_email_status(email)

assert (
"mork.celery.deletion_tasks",
"mork.celery.tasks.deletion",
logging.WARNING,
"Mork DB - No user found with email='[email protected]' for deletion",
) in caplog.record_tuples
Expand All @@ -230,7 +234,7 @@ class MockMorkDB:
session = db_session

EmailStatusFactory._meta.sqlalchemy_session = db_session
monkeypatch.setattr("mork.celery.deletion_tasks.MorkDB", MockMorkDB)
monkeypatch.setattr("mork.celery.tasks.deletion.MorkDB", MockMorkDB)

email = "[email protected]"
EmailStatusFactory.create(email=email)
Expand All @@ -240,7 +244,7 @@ class MockMorkDB:
delete_email_status(email)

assert (
"mork.celery.deletion_tasks",
"mork.celery.tasks.deletion",
logging.ERROR,
"Mork DB - Failed to delete user with email='[email protected]':"
" An error occurred",
Expand Down
Loading

0 comments on commit 2f7c549

Please sign in to comment.