feat: purge_personal_api_key_events() task (#7485)
* feat: purge_personal_api_key_events() task * feat: log number of events purged * test: test new task * fix: name task properly * chore: create daily PeriodicTask * chore: remove old management command * chore: remove tests of old command * test: finish removing now-empty tests.py
This commit is contained in:
Jennifer Richards
GitHub
parent
607a5c84b2
commit
020bdeb058
|
|
@ -36,6 +36,3 @@ $DTDIR/ietf/manage.py populate_yang_model_dirs -v0
|
|||
|
||||
# Re-run yang checks on active documents
|
||||
$DTDIR/ietf/manage.py run_yang_model_checks -v0
|
||||
|
||||
# Purge older PersonApiKeyEvents
|
||||
$DTDIR/ietf/manage.py purge_old_personal_api_key_events 14
|
||||
|
|
|
|||
|
|
@ -1,64 +0,0 @@
|
|||
# Copyright The IETF Trust 2021, All Rights Reserved
|
||||
# -*- coding: utf-8 -*-
|
||||
|
||||
from datetime import timedelta
|
||||
from django.core.management.base import BaseCommand, CommandError
|
||||
from django.db.models import Max, Min
|
||||
from django.utils import timezone
|
||||
|
||||
from ietf.person.models import PersonApiKeyEvent
|
||||
|
||||
|
||||
class Command(BaseCommand):
|
||||
help = 'Purge PersonApiKeyEvent instances older than KEEP_DAYS days'
|
||||
|
||||
def add_arguments(self, parser):
|
||||
parser.add_argument('keep_days', type=int,
|
||||
help='Delete events older than this many days')
|
||||
parser.add_argument('-n', '--dry-run', action='store_true', default=False,
|
||||
help="Don't delete events, just show what would be done")
|
||||
|
||||
|
||||
def handle(self, *args, **options):
|
||||
keep_days = options['keep_days']
|
||||
dry_run = options['dry_run']
|
||||
verbosity = options.get("verbosity", 1)
|
||||
|
||||
def _format_count(count, unit='day'):
|
||||
return '{} {}{}'.format(count, unit, ('' if count == 1 else 's'))
|
||||
|
||||
if keep_days < 0:
|
||||
raise CommandError('Negative keep_days not allowed ({} was specified)'.format(keep_days))
|
||||
|
||||
if verbosity > 1:
|
||||
self.stdout.write('purge_old_personal_api_key_events: Finding events older than {}\n'.format(_format_count(keep_days)))
|
||||
if dry_run:
|
||||
self.stdout.write('Dry run requested, records will not be deleted\n')
|
||||
self.stdout.flush()
|
||||
|
||||
now = timezone.now()
|
||||
old_events = PersonApiKeyEvent.objects.filter(
|
||||
time__lt=now - timedelta(days=keep_days)
|
||||
)
|
||||
|
||||
stats = old_events.aggregate(Min('time'), Max('time'))
|
||||
old_count = old_events.count()
|
||||
if old_count == 0:
|
||||
if verbosity > 1:
|
||||
self.stdout.write('No events older than {} found\n'.format(_format_count(keep_days)))
|
||||
return
|
||||
|
||||
oldest_date = stats['time__min']
|
||||
oldest_ago = now - oldest_date
|
||||
newest_date = stats['time__max']
|
||||
newest_ago = now - newest_date
|
||||
|
||||
action_fmt = 'Would delete {}\n' if dry_run else 'Deleting {}\n'
|
||||
if verbosity > 1:
|
||||
self.stdout.write(action_fmt.format(_format_count(old_count, 'event')))
|
||||
self.stdout.write(' Oldest at {} ({} ago)\n'.format(oldest_date, _format_count(oldest_ago.days)))
|
||||
self.stdout.write(' Most recent at {} ({} ago)\n'.format(newest_date, _format_count(newest_ago.days)))
|
||||
self.stdout.flush()
|
||||
|
||||
if not dry_run:
|
||||
old_events.delete()
|
||||
|
|
@ -1,122 +0,0 @@
|
|||
# Copyright The IETF Trust 2021, All Rights Reserved
|
||||
# -*- coding: utf-8 -*-
|
||||
|
||||
import datetime
|
||||
from io import StringIO
|
||||
|
||||
from django.core.management import call_command, CommandError
|
||||
from django.utils import timezone
|
||||
|
||||
from ietf.person.factories import PersonApiKeyEventFactory
|
||||
from ietf.person.models import PersonApiKeyEvent, PersonEvent
|
||||
from ietf.utils.test_utils import TestCase
|
||||
|
||||
|
||||
class CommandTests(TestCase):
|
||||
@staticmethod
|
||||
def _call_command(command_name, *args, **options):
|
||||
out = StringIO()
|
||||
options['stdout'] = out
|
||||
call_command(command_name, *args, **options)
|
||||
return out.getvalue()
|
||||
|
||||
def _assert_purge_results(self, cmd_output, expected_delete_count, expected_kept_events):
|
||||
self.assertNotIn('Dry run requested', cmd_output)
|
||||
if expected_delete_count == 0:
|
||||
delete_text = 'No events older than'
|
||||
else:
|
||||
delete_text = 'Deleting {} event'.format(expected_delete_count)
|
||||
self.assertIn(delete_text, cmd_output)
|
||||
self.assertCountEqual(
|
||||
PersonApiKeyEvent.objects.all(),
|
||||
expected_kept_events,
|
||||
'Wrong events were deleted'
|
||||
)
|
||||
|
||||
def _assert_purge_dry_run_results(self, cmd_output, expected_delete_count, expected_kept_events):
|
||||
self.assertIn('Dry run requested', cmd_output)
|
||||
if expected_delete_count == 0:
|
||||
delete_text = 'No events older than'
|
||||
else:
|
||||
delete_text = 'Would delete {} event'.format(expected_delete_count)
|
||||
self.assertIn(delete_text, cmd_output)
|
||||
self.assertCountEqual(
|
||||
PersonApiKeyEvent.objects.all(),
|
||||
expected_kept_events,
|
||||
'Events were deleted when dry-run option was used'
|
||||
)
|
||||
|
||||
def test_purge_old_personal_api_key_events(self):
|
||||
keep_days = 10
|
||||
|
||||
# Remember how many PersonEvents were present so we can verify they're cleaned up properly.
|
||||
personevents_before = PersonEvent.objects.count()
|
||||
|
||||
now = timezone.now()
|
||||
# The first of these events will be timestamped a fraction of a second more than keep_days
|
||||
# days ago by the time we call the management command, so will just barely chosen for purge.
|
||||
old_events = [
|
||||
PersonApiKeyEventFactory(time=now - datetime.timedelta(days=n))
|
||||
for n in range(keep_days, 2 * keep_days + 1)
|
||||
]
|
||||
num_old_events = len(old_events)
|
||||
|
||||
recent_events = [
|
||||
PersonApiKeyEventFactory(time=now - datetime.timedelta(days=n))
|
||||
for n in range(0, keep_days)
|
||||
]
|
||||
# We did not create recent_event timestamped exactly keep_days ago because it would
|
||||
# be treated as an old_event by the management command. Create an event a few seconds
|
||||
# on the "recent" side of keep_days old to test the threshold.
|
||||
recent_events.append(
|
||||
PersonApiKeyEventFactory(
|
||||
time=now + datetime.timedelta(seconds=3) - datetime.timedelta(days=keep_days)
|
||||
)
|
||||
)
|
||||
num_recent_events = len(recent_events)
|
||||
|
||||
# call with dry run
|
||||
output = self._call_command('purge_old_personal_api_key_events', str(keep_days), '--dry-run', '-v2')
|
||||
self._assert_purge_dry_run_results(output, num_old_events, old_events + recent_events)
|
||||
|
||||
# call for real
|
||||
output = self._call_command('purge_old_personal_api_key_events', str(keep_days), '-v2')
|
||||
self._assert_purge_results(output, num_old_events, recent_events)
|
||||
self.assertEqual(PersonEvent.objects.count(), personevents_before + num_recent_events,
|
||||
'PersonEvents were not cleaned up properly')
|
||||
|
||||
# repeat - there should be nothing left to delete
|
||||
output = self._call_command('purge_old_personal_api_key_events', '--dry-run', str(keep_days), '-v2')
|
||||
self._assert_purge_dry_run_results(output, 0, recent_events)
|
||||
|
||||
output = self._call_command('purge_old_personal_api_key_events', str(keep_days), '-v2')
|
||||
self._assert_purge_results(output, 0, recent_events)
|
||||
self.assertEqual(PersonEvent.objects.count(), personevents_before + num_recent_events,
|
||||
'PersonEvents were not cleaned up properly')
|
||||
|
||||
# and now delete the remaining events
|
||||
output = self._call_command('purge_old_personal_api_key_events', '0', '-v2')
|
||||
self._assert_purge_results(output, num_recent_events, [])
|
||||
self.assertEqual(PersonEvent.objects.count(), personevents_before,
|
||||
'PersonEvents were not cleaned up properly')
|
||||
|
||||
def test_purge_old_personal_api_key_events_rejects_invalid_arguments(self):
|
||||
"""The purge_old_personal_api_key_events command should reject invalid arguments"""
|
||||
event = PersonApiKeyEventFactory(time=timezone.now() - datetime.timedelta(days=30))
|
||||
|
||||
with self.assertRaises(CommandError):
|
||||
self._call_command('purge_old_personal_api_key_events')
|
||||
|
||||
with self.assertRaises(CommandError):
|
||||
self._call_command('purge_old_personal_api_key_events', '-15')
|
||||
|
||||
with self.assertRaises(CommandError):
|
||||
self._call_command('purge_old_personal_api_key_events', '15.3')
|
||||
|
||||
with self.assertRaises(CommandError):
|
||||
self._call_command('purge_old_personal_api_key_events', '15', '15')
|
||||
|
||||
with self.assertRaises(CommandError):
|
||||
self._call_command('purge_old_personal_api_key_events', 'abc', '15')
|
||||
|
||||
self.assertCountEqual(PersonApiKeyEvent.objects.all(), [event])
|
||||
20
ietf/person/tasks.py
Normal file
20
ietf/person/tasks.py
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
# Copyright The IETF Trust 2024, All Rights Reserved
|
||||
#
|
||||
# Celery task definitions
|
||||
#
|
||||
import datetime
|
||||
|
||||
from celery import shared_task
|
||||
from django.utils import timezone
|
||||
|
||||
from ietf.utils import log
|
||||
from .models import PersonApiKeyEvent
|
||||
|
||||
|
||||
@shared_task
|
||||
def purge_personal_api_key_events_task(keep_days):
|
||||
keep_since = timezone.now() - datetime.timedelta(days=keep_days)
|
||||
old_events = PersonApiKeyEvent.objects.filter(time__lt=keep_since)
|
||||
count = len(old_events)
|
||||
old_events.delete()
|
||||
log.log(f"Deleted {count} PersonApiKeyEvents older than {keep_since}")
|
||||
|
|
@ -4,6 +4,7 @@
|
|||
|
||||
import datetime
|
||||
import json
|
||||
import mock
|
||||
|
||||
from io import StringIO, BytesIO
|
||||
from PIL import Image
|
||||
|
|
@ -25,8 +26,9 @@ from ietf.group.models import Group
|
|||
from ietf.nomcom.models import NomCom
|
||||
from ietf.nomcom.test_data import nomcom_test_data
|
||||
from ietf.nomcom.factories import NomComFactory, NomineeFactory, NominationFactory, FeedbackFactory, PositionFactory
|
||||
from ietf.person.factories import EmailFactory, PersonFactory
|
||||
from ietf.person.models import Person, Alias
|
||||
from ietf.person.factories import EmailFactory, PersonFactory, PersonApiKeyEventFactory
|
||||
from ietf.person.models import Person, Alias, PersonApiKeyEvent
|
||||
from ietf.person.tasks import purge_personal_api_key_events_task
|
||||
from ietf.person.utils import (merge_persons, determine_merge_order, send_merge_notification,
|
||||
handle_users, get_extra_primary, dedupe_aliases, move_related_objects, merge_nominees,
|
||||
handle_reviewer_settings, get_dots)
|
||||
|
|
@ -450,3 +452,16 @@ class PersonUtilsTests(TestCase):
|
|||
self.assertEqual(get_dots(ncmember),['nomcom'])
|
||||
ncchair = RoleFactory(group__acronym='nomcom2020',group__type_id='nomcom',name_id='chair').person
|
||||
self.assertEqual(get_dots(ncchair),['nomcom'])
|
||||
|
||||
|
||||
class TaskTests(TestCase):
|
||||
@mock.patch("ietf.person.tasks.log.log")
|
||||
def test_purge_personal_api_key_events_task(self, mock_log):
|
||||
now = timezone.now()
|
||||
old_event = PersonApiKeyEventFactory(time=now - datetime.timedelta(days=1, minutes=1))
|
||||
young_event = PersonApiKeyEventFactory(time=now - datetime.timedelta(days=1, minutes=-1))
|
||||
purge_personal_api_key_events_task(keep_days=1)
|
||||
self.assertFalse(PersonApiKeyEvent.objects.filter(pk=old_event.pk).exists())
|
||||
self.assertTrue(PersonApiKeyEvent.objects.filter(pk=young_event.pk).exists())
|
||||
self.assertTrue(mock_log.called)
|
||||
self.assertIn("Deleted 1", mock_log.call_args[0][0])
|
||||
|
|
|
|||
|
|
@ -241,6 +241,17 @@ class Command(BaseCommand):
|
|||
),
|
||||
)
|
||||
|
||||
PeriodicTask.objects.get_or_create(
|
||||
name="Purge old personal API key events",
|
||||
task="ietf.person.tasks.purge_personal_api_key_events_task",
|
||||
kwargs=json.dumps(dict(keep_days=14)),
|
||||
defaults=dict(
|
||||
enabled=False,
|
||||
crontab=self.crontabs["daily"],
|
||||
description="Purge PersonApiKeyEvent instances older than 14 days",
|
||||
),
|
||||
)
|
||||
|
||||
def show_tasks(self):
|
||||
for label, crontab in self.crontabs.items():
|
||||
tasks = PeriodicTask.objects.filter(crontab=crontab).order_by(
|
||||
|
|
|
|||
Loading…
Reference in a new issue