Skip to content

Commit 448c9c5

Browse files
marcuamarcua
authored
SanityBot (#435)
* SanityBot models * Admin for SanityCheck * v1 SanityBot implementation * Refactor to support repetition seconds * Sanitybot tests * sort * sort * Disable isort * clean up workflows * Clean up complete project logic * Address code review * Remove prints from tests * docs * merge conflict
1 parent e2bc33f commit 448c9c5

16 files changed

+418
-7
lines changed

‎Makefile

+3-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ clean:
99
find . -name '*~' -delete
1010

1111
lint:
12-
flake8 .
12+
# TODO(marcua): reenable isort when it works the same in dev and
13+
# on CircleCI
14+
flake8 . # && isort --check-only --recursive .
1315
gulp lint --production
1416
!(find . -wholename '**migrations**/*.py' -print0 | xargs -0 grep 'RemoveField\|DeleteModel\|AlterModelTable\|AlterUniqueTogether\|RunSQL\|RunPython\|SeparateDatabaseAndState' | grep -v '# manually-reviewed') || { echo "Migrations need to be manually reviewed!"; exit 1; }
1517
!(cd example_project && python3 manage.py makemigrations --dry-run --exit) || { printf "Migrations need to be created. Try: \n\t python3 manage.py makemigrations\n"; exit 1; }

‎docs/source/bots.rst

+55
Original file line numberDiff line numberDiff line change
@@ -202,3 +202,58 @@ To restaff this task you can type::
202202

203203
This will offer the task again to eligible experts, and once a new expert
204204
accepts, ``joshblum`` will be removed and the new expert will be added.
205+
206+
*********
207+
SanityBot
208+
*********
209+
210+
Setup
211+
-----
212+
213+
``SanityBot`` periodically looks at the state of a project and reminds
214+
the project team about various things that seem off. For details and
215+
motivation, see the `original project description
216+
<https://github.com/b12io/orchestra/issues/434>`_. ``SanityBot``
217+
currently warns project team members in the project team's Slack
218+
channel.
219+
220+
Project Configuration
221+
=====================
222+
223+
To specify which sanity checks to run, and how frequently to run them,
224+
update ``version.json`` for the workflow you are sanity-checking with
225+
an optional ``sanity_checks`` entry. As an example::
226+
227+
228+
[...workflow definition...]
229+
"sanity_checks": {
230+
"sanity_check_function": {
231+
"path": "path.to.sanity.check.function"
232+
},
233+
"check_configurations": {
234+
"check_slug1": {
235+
"handlers": [{"type": "slack_project_channel", "message": "<message here>", "steps": ["step_slug1", ...]}],
236+
"repetition_seconds": 3600
237+
},
238+
...
239+
},
240+
}
241+
...
242+
243+
244+
Here's a walkthrough of the configuration above:
245+
246+
* ``sanity_check_function`` is called periodically and generates SanityCheck objects. The function prototype is ``def sanity_check_function(project: Project) -> List[SanityCheck]:``.
247+
* ``check_configurations`` maps ``SanityCheck.check_slug`` values to a configuration, which consists of a list of handlers and a repetition interval.
248+
* in v1, the only handler is ``slack_project_channel``, which messages the team slack project, tagging the experts assigned to the tasks specified by in steps.
249+
* An optional ``repetition_seconds`` contains the number of seconds to wait before re-issuing/re-handling a ``SanityCheck``. If ``repetition_seconds`` does not appear in the map, that ``SanityCheck`` is not repeated.
250+
251+
252+
Scheduling function
253+
===================
254+
To operationalize ``SanityBot``, you should call
255+
``orchestra.bots.sanitybot.create_and_handle_sanity_checks`` through
256+
``cron`` or some other scheduling utility. This function will look at
257+
all active projects with ``sanity_checks`` in their workflow
258+
definitions, and call the appropriate ``sanity_check_function`` to
259+
trigger sanity checks.

‎orchestra/admin.py

+13
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from orchestra.models import Iteration
1717
from orchestra.models import PayRate
1818
from orchestra.models import Project
19+
from orchestra.models import SanityCheck
1920
from orchestra.models import StaffBotRequest
2021
from orchestra.models import StaffingRequestInquiry
2122
from orchestra.models import StaffingResponse
@@ -87,6 +88,18 @@ class ProjectAdmin(admin.ModelAdmin):
8788
list_filter = ('workflow_version',)
8889

8990

91+
@admin.register(SanityCheck)
92+
class SanityCheckAdmin(AjaxSelectAdmin):
93+
form = make_ajax_form(SanityCheck, {
94+
'project': 'projects',
95+
})
96+
list_display = ('id', 'created_at', 'project', 'check_slug', 'handled_at')
97+
ordering = ('-created_at',)
98+
search_fields = (
99+
'project__short_description', 'check_slug')
100+
list_filter = ('project__workflow_version',)
101+
102+
90103
@admin.register(Step)
91104
class StepAdmin(admin.ModelAdmin):
92105
list_display = (

‎orchestra/bots/sanitybot.py

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
from django.db.models import Max
2+
from django.utils import timezone
3+
from pydoc import locate
4+
5+
from orchestra.core.errors import SanityBotError
6+
from orchestra.models import Project
7+
from orchestra.models import SanityCheck
8+
from orchestra.models import WorkflowVersion
9+
from orchestra.utils.notifications import message_experts_slack_group
10+
from orchestra.utils.project_properties import incomplete_projects
11+
12+
13+
def _handle(project, sanity_check, handler):
14+
handler_type = handler.get('type')
15+
handler_message = handler.get('message')
16+
handler_steps = handler.get('steps')
17+
if (handler_type != 'slack_project_channel' or
18+
not handler_message or not handler_steps):
19+
raise SanityBotError('Invalid handler: {}'.format(handler))
20+
tasks = (
21+
task for task in project.tasks.all()
22+
if task.step.slug in handler_steps)
23+
usernames = {
24+
assignment.worker.formatted_slack_username()
25+
for task in tasks
26+
for assignment in task.assignments.all()
27+
if assignment and assignment.worker
28+
}
29+
message = '{}: {}'.format(' '.join(usernames), handler_message)
30+
message_experts_slack_group(
31+
project.slack_group_id, message)
32+
33+
34+
def _filter_checks(project, checks, check_configurations):
35+
latest_check_creation = {
36+
check['check_slug']: check['max_created_at']
37+
for check in (SanityCheck.objects
38+
.filter(project=project)
39+
.values('check_slug')
40+
.annotate(max_created_at=Max('created_at')))}
41+
for check in checks:
42+
max_created_at = latest_check_creation.get(check.check_slug)
43+
seconds = (
44+
check_configurations.get(check.check_slug, {})
45+
.get('repetition_seconds'))
46+
now = timezone.now()
47+
if (max_created_at is None or
48+
((seconds is not None) and
49+
((now - max_created_at).total_seconds() > seconds))):
50+
yield check
51+
52+
53+
def _handle_sanity_checks(project, sanity_checks, check_configurations):
54+
sanity_checks = _filter_checks(
55+
project, sanity_checks, check_configurations)
56+
for sanity_check in sanity_checks:
57+
config = check_configurations.get(sanity_check.check_slug)
58+
if config is None:
59+
raise SanityBotError(
60+
'No configuration for {}'.format(sanity_check.check_slug))
61+
handlers = config.get('handlers')
62+
if handlers is None:
63+
raise SanityBotError(
64+
'No handlers for {}'.format(sanity_check.check_slug))
65+
for handler in handlers:
66+
_handle(project, sanity_check, handler)
67+
sanity_check.handled_at = timezone.now()
68+
sanity_check.project = project
69+
sanity_check.save()
70+
71+
72+
def create_and_handle_sanity_checks():
73+
workflow_versions = WorkflowVersion.objects.all()
74+
for project in incomplete_projects(
75+
Project.objects.filter(
76+
workflow_version__in=workflow_versions)):
77+
sanity_checks = project.workflow_version.sanity_checks
78+
sanity_check_path = (sanity_checks
79+
.get('sanity_check_function', {})
80+
.get('path'))
81+
check_configurations = sanity_checks.get('check_configurations')
82+
if sanity_check_path and check_configurations:
83+
sanity_check_function = locate(sanity_check_path)
84+
sanity_checks = sanity_check_function(project)
85+
_handle_sanity_checks(
86+
project, sanity_checks, check_configurations)

‎orchestra/bots/tests/test_sanitybot.py

+85
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
from collections import Counter
2+
from datetime import timedelta
3+
from unittest.mock import patch
4+
5+
from orchestra.bots.sanitybot import create_and_handle_sanity_checks
6+
from orchestra.models import SanityCheck
7+
from orchestra.tests.helpers import OrchestraTestCase
8+
from orchestra.tests.helpers.fixtures import setup_models
9+
from orchestra.utils.task_lifecycle import create_subsequent_tasks
10+
11+
12+
class SanityBotTestCase(OrchestraTestCase):
13+
14+
def setUp(self):
15+
super().setUp()
16+
setup_models(self)
17+
18+
@patch('orchestra.bots.sanitybot.message_experts_slack_group')
19+
def test_create_and_handle_sanity_checks(self, mock_slack):
20+
# Initialize relevant project.
21+
create_subsequent_tasks(self.projects['sanitybot'])
22+
# Initialize irrelevant project.
23+
create_subsequent_tasks(self.projects['test_human_and_machine'])
24+
25+
# No sanity checks exist, and Slack hasn't received a message.
26+
sanity_checks = SanityCheck.objects.all()
27+
self.assertEqual(sanity_checks.count(), 0)
28+
self.assertEqual(mock_slack.call_count, 0)
29+
30+
create_and_handle_sanity_checks()
31+
# Of the four sanity checks for this project, three were
32+
# triggered by orchestra.tests.helpers.workflow.check_project.
33+
sanity_checks = SanityCheck.objects.all()
34+
self.assertEqual(
35+
dict(Counter(sanity_checks.values_list('check_slug', flat=True))),
36+
{
37+
'frequently_repeating_check': 1,
38+
'infrequently_repeating_check': 1,
39+
'onetime_check': 1
40+
})
41+
for sanity_check in sanity_checks:
42+
self.assertEqual(
43+
sanity_check.project.workflow_version.slug,
44+
'sanitybot_workflow')
45+
# Three slack messages for three sanity checks.
46+
self.assertEqual(mock_slack.call_count, 3)
47+
48+
# Too little time has passed, so we expect no new sanity checks.
49+
create_and_handle_sanity_checks()
50+
# Still only three sanity check exists for the relevant project.
51+
sanity_checks = SanityCheck.objects.all()
52+
self.assertEqual(sanity_checks.count(), 3)
53+
# Slack didn't get called again.
54+
self.assertEqual(mock_slack.call_count, 3)
55+
56+
# Mark the sanity checks as having happened two days ago. Only
57+
# the `frequently_repeating_check` should trigger.
58+
for sanity_check in sanity_checks:
59+
sanity_check.created_at = (
60+
sanity_check.created_at - timedelta(days=2))
61+
sanity_check.save()
62+
create_and_handle_sanity_checks()
63+
# Look for the three old sanity checks and a new
64+
# `frequently_repeating_check`.
65+
sanity_checks = SanityCheck.objects.all()
66+
self.assertEqual(
67+
dict(Counter(sanity_checks.values_list('check_slug', flat=True))),
68+
{
69+
'frequently_repeating_check': 2,
70+
'infrequently_repeating_check': 1,
71+
'onetime_check': 1
72+
})
73+
for sanity_check in sanity_checks:
74+
self.assertEqual(
75+
sanity_check.project.workflow_version.slug,
76+
'sanitybot_workflow')
77+
# Slack got called another time.
78+
self.assertEqual(mock_slack.call_count, 4)
79+
80+
create_and_handle_sanity_checks()
81+
# Still only four sanity checks exists for the relevant project.
82+
sanity_checks = SanityCheck.objects.all()
83+
self.assertEqual(sanity_checks.count(), 4)
84+
# Slack didn't get called again.
85+
self.assertEqual(mock_slack.call_count, 4)

‎orchestra/core/errors.py

+4
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ class ReviewPolicyError(Exception):
3030
pass
3131

3232

33+
class SanityBotError(Exception):
34+
pass
35+
36+
3337
class S3UploadError(Exception):
3438
pass
3539

‎orchestra/migrations/0075_add_sanity_checks.py

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# -*- coding: utf-8 -*-
2+
# Generated by Django 1.9.13 on 2018-01-21 15:41
3+
from __future__ import unicode_literals
4+
5+
from django.db import migrations, models
6+
import django.db.models.deletion
7+
import django.utils.timezone
8+
import jsonfield.fields
9+
import orchestra.models.core.mixins
10+
import orchestra.utils.models
11+
12+
13+
class Migration(migrations.Migration):
14+
15+
dependencies = [
16+
('orchestra', '0074_auto_20180116_1523'),
17+
]
18+
19+
operations = [
20+
migrations.CreateModel(
21+
name='SanityCheck',
22+
fields=[
23+
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
24+
('created_at', models.DateTimeField(default=django.utils.timezone.now)),
25+
('is_deleted', models.BooleanField(default=False)),
26+
('handled_at', models.DateTimeField(blank=True, null=True)),
27+
('check_slug', models.CharField(max_length=200)),
28+
('project', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='sanity_checks', to='orchestra.Project')),
29+
],
30+
bases=(orchestra.models.core.mixins.SanityCheckMixin, orchestra.utils.models.DeleteMixin, models.Model),
31+
),
32+
migrations.AddField(
33+
model_name='workflowversion',
34+
name='sanity_checks',
35+
field=jsonfield.fields.JSONField(default={}),
36+
),
37+
]

‎orchestra/models/__init__.py

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from orchestra.models.core.models import Workflow
66
from orchestra.models.core.models import WorkflowVersion
77
from orchestra.models.core.models import Certification
8+
from orchestra.models.core.models import SanityCheck
89
from orchestra.models.core.models import Step
910
from orchestra.models.core.models import Worker
1011
from orchestra.models.core.models import WorkerCertification
@@ -26,6 +27,7 @@
2627
'Workflow',
2728
'WorkflowVersion',
2829
'Certification',
30+
'SanityCheck',
2931
'Step',
3032
'Worker',
3133
'WorkerCertification',

‎orchestra/models/core/mixins.py

+10
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,16 @@ def __str__(self):
2323
return '{} - {}'.format(self.slug, self.workflow.slug)
2424

2525

26+
class SanityCheckMixin(object):
27+
28+
def __str__(self):
29+
return '{} - {} (created: {}, handled: {})'.format(
30+
self.project,
31+
self.check_slug,
32+
self.created_at,
33+
self.handled_at)
34+
35+
2636
class StepMixin(object):
2737

2838
def __str__(self):

0 commit comments

Comments
 (0)