diff --git a/actioner/clients.py b/actioner/clients.py index ac47fd3..aa77025 100644 --- a/actioner/clients.py +++ b/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) diff --git a/actioner/scheduler/todoist_assigned_issues.py b/actioner/scheduler/todoist_assigned_issues.py index 656fd24..9d1cb4e 100644 --- a/actioner/scheduler/todoist_assigned_issues.py +++ b/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]) diff --git a/actioner/scheduler/todoist_repo_prs.py b/actioner/scheduler/todoist_repo_prs.py index dec982d..92bda3b 100644 --- a/actioner/scheduler/todoist_repo_prs.py +++ b/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 ( - my_review.commit_id == pr.head.sha - and not existing_task["checked"] + if my_review.commit_id == pr.head.sha and not is_task_completed( + existing_task ): - 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)) diff --git a/actioner/utils/todoist.py b/actioner/utils/todoist.py new file mode 100644 index 0000000..c4abeee --- /dev/null +++ b/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 diff --git a/actioner/web/healthcheck.py b/actioner/web/healthcheck.py index 07df402..a09d0e9 100644 --- a/actioner/web/healthcheck.py +++ b/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()} diff --git a/tests/test_utils/test_init.py b/tests/test_utils/test_init.py index bc4dbbe..d08d131 100644 --- a/tests/test_utils/test_init.py +++ b/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"]}