🚚(celery) rename celery tasks by adding a _task suffix

src/app/mork/api/models.py

@@ -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
@@ -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):
@@ -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,
 }

src/app/mork/celery/celery_app.py

@@ -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

src/app/mork/celery/tasks/__init__.py

@@ -0,0 +1 @@
+# noqa: D104

src/app/mork/celery/deletion_tasks.py → src/app/mork/celery/tasks/deletion.py

@@ -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
@@ -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()
 

src/app/mork/celery/emailing_tasks.py → src/app/mork/celery/tasks/emailing.py

@@ -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()
 
@@ -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")

src/app/mork/tests/api/test_tasks.py

@@ -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
 ):
@@ -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

src/app/mork/tests/celery/test_deletion_task.py

@@ -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
@@ -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]"),
  ]
  )
 
@@ -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,
@@ -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,
@@ -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):
@@ -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)
@@ -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]"
 
@@ -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
@@ -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)
@@ -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",