From 2ed77eb27a67c4cffd7fb9c6db363a143168f53d Mon Sep 17 00:00:00 2001
From: Benjamin Bertrand <benjamin.bertrand@esss.se>
Date: Fri, 28 Sep 2018 14:48:35 +0200
Subject: [PATCH] Do not take into account old deferred tasks
A deferred task will remain deferred forever if the task it depends on
fails. When checking for waiting tasks, we should ignore deferred tasks
that are too old.
"Too old" is currently set to 30 minutes. We shouldn't have tasks that
take that long. We might want to shorten that even a bit (to not block
new identical tasks to be run) but if a task failed there is probably
something to be done.
JIRA INFRA-559 #action In Progress
---
app/models.py | 19 +++++++++-
tests/functional/conftest.py | 1 +
tests/functional/factories.py | 15 ++++++--
tests/functional/test_models.py | 65 +++++++++++++++++++++++++++++++++
4 files changed, 94 insertions(+), 6 deletions(-)
diff --git a/app/models.py b/app/models.py
index 69217b7..4832add 100644
--- a/app/models.py
+++ b/app/models.py
@@ -9,6 +9,7 @@ This module implements the models used in the app.
:license: BSD 2-Clause, see LICENSE for more details.
"""
+import datetime
import ipaddress
import string
import qrcode
@@ -283,10 +284,24 @@ class User(db.Model, UserMixin):
return Task.query.filter_by(name=name, status=JobStatus.STARTED).first()
def is_task_waiting(self, name):
- """Return True if a <name> task is waiting (queued or deferred)"""
+ """Return True if a <name> task is waiting
+
+ Waiting means:
+ - queued
+ - deferred if not older than 30 minutes
+
+ A deferred task can stay deferred forever if the task it depends on fails.
+ """
+ thirty_minutes_ago = datetime.datetime.utcnow() - datetime.timedelta(minutes=30)
count = (
Task.query.filter_by(name=name)
- .filter(Task.status.in_([JobStatus.DEFERRED, JobStatus.QUEUED]))
+ .filter(
+ (Task.status == JobStatus.QUEUED)
+ | (
+ (Task.status == JobStatus.DEFERRED)
+ & (Task.created_at > thirty_minutes_ago)
+ )
+ )
.count()
)
return count > 0
diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
index 0774d78..7bace7f 100644
--- a/tests/functional/conftest.py
+++ b/tests/functional/conftest.py
@@ -34,6 +34,7 @@ register(factories.MacFactory)
register(factories.DomainFactory)
register(factories.CnameFactory)
register(factories.TagFactory)
+register(factories.TaskFactory)
@pytest.fixture(scope="session")
diff --git a/tests/functional/factories.py b/tests/functional/factories.py
index ace920c..04805e5 100644
--- a/tests/functional/factories.py
+++ b/tests/functional/factories.py
@@ -11,14 +11,10 @@ This module defines models factories.
"""
import ipaddress
import factory
-from faker import Factory as FakerFactory
from app import models
from . import common
-faker = FakerFactory.create()
-
-
class UserFactory(factory.alchemy.SQLAlchemyModelFactory):
class Meta:
model = models.User
@@ -210,3 +206,14 @@ class TagFactory(factory.alchemy.SQLAlchemyModelFactory):
sqlalchemy_session_persistence = "commit"
name = factory.Sequence(lambda n: f"Tag{n}")
+
+
+class TaskFactory(factory.alchemy.SQLAlchemyModelFactory):
+ class Meta:
+ model = models.Task
+ sqlalchemy_session = common.Session
+ sqlalchemy_session_persistence = "commit"
+
+ id = factory.Faker("uuid4")
+ name = factory.Sequence(lambda n: f"task{n}")
+ user = factory.SubFactory(UserFactory)
diff --git a/tests/functional/test_models.py b/tests/functional/test_models.py
index 96aa143..8bbdfae 100644
--- a/tests/functional/test_models.py
+++ b/tests/functional/test_models.py
@@ -9,6 +9,7 @@ This module defines models tests.
:license: BSD 2-Clause, see LICENSE for more details.
"""
+import datetime
import ipaddress
import pytest
from wtforms import ValidationError
@@ -287,3 +288,67 @@ def test_ansible_dynamic_network_scope_group(
assert group_s1.hosts == [host1_s1, host2_s1]
assert group_s2.hosts == [host1_s2]
assert group_s3.hosts == []
+
+
+@pytest.mark.parametrize("status", [None, "FINISHED", "FAILED", "STARTED"])
+def test_no_task_waiting(status, user, task_factory):
+ if status is None:
+ task_factory(name="my-task", status=None)
+ else:
+ task_factory(name="my-task", status=models.JobStatus[status])
+ assert not user.is_task_waiting("my-task")
+
+
+@pytest.mark.parametrize("status", ["QUEUED", "DEFERRED"])
+def test_task_waiting(status, user, task_factory):
+ task_factory(name="my-task", status=models.JobStatus[status])
+ assert user.is_task_waiting("my-task")
+
+
+@pytest.mark.parametrize("minutes", [5, 10, 29])
+def test_task_waiting_with_recent_deferred(minutes, user, task_factory):
+ minutes_ago = datetime.datetime.utcnow() - datetime.timedelta(minutes=minutes)
+ task_factory(
+ created_at=minutes_ago, name="my-task", status=models.JobStatus.DEFERRED
+ )
+ assert user.is_task_waiting("my-task")
+
+
+@pytest.mark.parametrize("minutes", [31, 60, 7200])
+def test_no_task_waiting_with_old_deferred(minutes, user, task_factory):
+ minutes_ago = datetime.datetime.utcnow() - datetime.timedelta(minutes=minutes)
+ task_factory(
+ created_at=minutes_ago, name="my-task", status=models.JobStatus.DEFERRED
+ )
+ assert not user.is_task_waiting("my-task")
+
+
+@pytest.mark.parametrize("minutes", [5, 30, 7200])
+def test_task_waiting_with_old_queued(minutes, user, task_factory):
+ minutes_ago = datetime.datetime.utcnow() - datetime.timedelta(minutes=minutes)
+ task_factory(created_at=minutes_ago, name="my-task", status=models.JobStatus.QUEUED)
+ assert user.is_task_waiting("my-task")
+
+
+def test_get_task_started_when_only_one(user, task_factory):
+ task_factory(name="my-task", status=models.JobStatus.QUEUED)
+ task = task_factory(name="my-task", status=models.JobStatus.STARTED)
+ task_factory(name="my-task", status=models.JobStatus.FINISHED)
+ task_factory(name="my-task", status=models.JobStatus.FAILED)
+ assert user.get_task_started("my-task") == task
+
+
+def test_get_task_started_when_several(user, task_factory):
+ # Only the first started task is returned
+ task1 = task_factory(name="my-task", status=models.JobStatus.STARTED)
+ task_factory(name="my-task", status=models.JobStatus.STARTED)
+ assert user.get_task_started("my-task") == task1
+
+
+def test_get_tasks_in_progress(user, task_factory):
+ task1 = task_factory(name="my-task", status=models.JobStatus.QUEUED)
+ task2 = task_factory(name="my-task", status=models.JobStatus.STARTED)
+ task_factory(name="my-task", status=models.JobStatus.FINISHED)
+ task_factory(name="my-task", status=models.JobStatus.FAILED)
+ task3 = task_factory(name="my-task", status=models.JobStatus.DEFERRED)
+ assert user.get_tasks_in_progress("my-task") == [task1, task2, task3]
--
GitLab