Create todoist api client each time, as it's not thread safe

actioner/clients.py

@@ -1,12 +1,13 @@
-import os
-import tempfile
-
 from github import Github
 from todoist import TodoistAPI
 
 from actioner.settings import GITHUB_TOKEN, TODOIST_TOKEN
 
 github = Github(GITHUB_TOKEN)
-todoist = TodoistAPI(
+
-    TODOIST_TOKEN, cache=os.path.join(tempfile.gettempdir(), "todoist-api")
+
-)
+def get_todoist_client():
+    """
+    The Todoist client isn't thread safe, so we need to create it each time we want to use it
+    """
+    return TodoistAPI(TODOIST_TOKEN)

actioner/scheduler/todoist_assigned_issues.py

@@ -2,9 +2,10 @@ import logging
 
 from github import Issue
 
-from actioner.clients import github, todoist
+from actioner.clients import get_todoist_client, github
 from actioner.utils import get_todoist_project_from_repo
 from actioner.utils.github import get_existing_task, get_issue_link
+from actioner.utils.todoist import is_task_completed
 
 REPOS = ["srobo/tasks", "srobo/core-team-minutes"]
 
@@ -23,6 +24,7 @@ def issue_to_task_name(issue: Issue) -> str:
 
 
 def todoist_assigned_issues():
+    todoist = get_todoist_client()
     me = github.get_user()
     todoist.projects.sync()
     todoist.items.sync()
@@ -46,7 +48,7 @@ def todoist_assigned_issues():
             elif (
                 issue.state == "closed"
                 and existing_task_id is not None
-                and not todoist.items.get_by_id(existing_task_id)["checked"]
+                and not is_task_completed(todoist.items.get_by_id(existing_task_id))
             ):
                 logger.info("Completing task for '{}'".format(issue.title))
                 todoist.items.complete([existing_task_id])
@@ -60,7 +62,7 @@ def todoist_assigned_issues():
                     )["id"]
                 existing_task = todoist.items.get_by_id(existing_task_id)
 
-                if existing_task["checked"]:
+                if is_task_completed(existing_task):
                     logger.info("Re-opening task '{}'".format(issue.title))
                     todoist.items.uncomplete([existing_task_id])
 

actioner/scheduler/todoist_repo_prs.py

@@ -2,9 +2,10 @@ import logging
 
 from github import PullRequest
 
-from actioner.clients import github, todoist
+from actioner.clients import get_todoist_client, github
 from actioner.utils import get_todoist_project_from_repo
 from actioner.utils.github import get_existing_task, get_issue_link
+from actioner.utils.todoist import is_task_completed
 
 logger = logging.getLogger(__name__)
 
@@ -23,6 +24,7 @@ def get_my_review(me, pr: PullRequest):
 
 
 def todoist_repo_prs():
+    todoist = get_todoist_client()
     me = github.get_user()
     todoist.projects.sync()
     todoist.items.sync()
@@ -39,7 +41,12 @@ def todoist_repo_prs():
 
             if pr.state == "closed" and existing_task_id:
                 my_review = get_my_review(me, pr)
-                if pr.merged and my_review and my_review.state == "APPROVED":
+                if (
+                    pr.merged
+                    and my_review
+                    and my_review.state == "APPROVED"
+                    and not is_task_completed(existing_task_id)
+                ):
                     logger.info("Completing task to review '{}'".format(pr.title))
                     todoist.items.complete([existing_task_id])
                 else:
@@ -56,13 +63,16 @@ def todoist_repo_prs():
                 existing_task = todoist.items.get_by_id(existing_task_id)
                 my_review = get_my_review(me, pr)
                 if existing_task_id and my_review:
-                    if (
+                    if my_review.commit_id == pr.head.sha and not is_task_completed(
-                        my_review.commit_id == pr.head.sha
+                        existing_task
-                        and not existing_task["checked"]
                     ):
-                        logger.info("Completing task to review '{}'".format(pr.title))
+                        logger.info(
+                            "Completing task to review '{}', because I already did it".format(
+                                pr.title
+                            )
+                        )
                         todoist.items.complete([existing_task_id])
-                    elif existing_task["checked"] and existing_task["checked"]:
+                    elif is_task_completed(existing_task):
                         logger.info("Re-opening task to review '{}'".format(pr.title))
                         todoist.items.uncomplete([existing_task_id])
                 existing_task.update(content=pr_to_task_name(pr))

actioner/utils/todoist.py

@@ -0,0 +1,11 @@
+from todoist.models import Item
+
+
+def is_task_completed(task: Item):
+    """
+    `Item` doesn't support `.get`, so re-implement it
+    """
+    try:
+        return task["checked"]
+    except KeyError:
+        return False

actioner/web/healthcheck.py

@@ -1,9 +1,10 @@
 from aiohttp import web
 
-from actioner.clients import github, todoist
+from actioner.clients import get_todoist_client, github
 
 
 async def healthcheck(request):
+    todoist = get_todoist_client()
     todoist.user.sync()
     return web.json_response(
         {"github": github.get_user().login, "todoist": todoist.user.get_id()}

tests/test_utils/test_init.py

@@ -1,4 +1,4 @@
-from actioner.clients import github, todoist
+from actioner.clients import get_todoist_client, github
 from actioner.utils import (
     GH_ORG_TO_TODOIST,
     GH_REPO_TO_TODOIST,
@@ -28,6 +28,7 @@ class TodoistProjectToRepoTestCase(BaseTestCase):
             github.get_organization(org)
 
     def test_project_exists(self):
+        todoist = get_todoist_client()
         project_ids = set(GH_ORG_TO_TODOIST.values()).union(GH_REPO_TO_TODOIST.values())
         todoist.projects.sync()
         todoist_project_ids = {project["id"] for project in todoist.state["projects"]}