| Message ID | 20180110004024.5551-2-stephen@that.guru |
|---|---|
| State | RFC |
| Headers | show |
| Series | Add 'Event.payload' field | expand |
Hi Stephen, This is a good idea and I'm happy for it to go in as-is, before the rest of the series. Reviewed-by: Daniel Axtens <dja@axtens.net> Regards, Daniel > Events are created for most state changes in Patchwork which means > they're fairly numerous. Start removing them after a (configurable) > interval. > > Signed-off-by: Stephen Finucane <stephen@that.guru> > --- > docs/deployment/configuration.rst | 7 +++++++ > docs/usage/overview.rst | 2 ++ > patchwork/management/commands/cron.py | 2 ++ > patchwork/notifications.py | 18 ++++++++++++++---- > patchwork/settings/base.py | 3 +++ > ...tomatically-remove-old-events-4ee222aaf4a6ea6c.yaml | 8 ++++++++ > 6 files changed, 36 insertions(+), 4 deletions(-) > create mode 100644 releasenotes/notes/automatically-remove-old-events-4ee222aaf4a6ea6c.yaml > > diff --git a/docs/deployment/configuration.rst b/docs/deployment/configuration.rst > index 34748563..3494bd39 100644 > --- a/docs/deployment/configuration.rst > +++ b/docs/deployment/configuration.rst > @@ -51,6 +51,13 @@ This is customizable on a per-user basis from the user configuration page. > This option was previously named ``DEFAULT_PATCHES_PER_PAGE``. It was > renamed as cover letters are now supported also. > > +``EVENT_VALIDITY_DAYS`` > +~~~~~~~~~~~~~~~~~~~~~~~ > + > +The number of days to retain :ref:`events` for. After this interval, the > +:ref:`cron management command <deployment-final-steps>` will delete the > +request. > + > ``CONFIRMATION_VALIDITY_DAYS`` > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > diff --git a/docs/usage/overview.rst b/docs/usage/overview.rst > index 802de2f1..7ac265f7 100644 > --- a/docs/usage/overview.rst > +++ b/docs/usage/overview.rst > @@ -207,6 +207,8 @@ To-do Lists > > Patchwork users can store a to-do list of patches. > > +.. _events: > + > Events > ------ > > diff --git a/patchwork/management/commands/cron.py b/patchwork/management/commands/cron.py > index 2be234b1..653fb0b3 100644 > --- a/patchwork/management/commands/cron.py > +++ b/patchwork/management/commands/cron.py > @@ -19,6 +19,7 @@ > > from django.core.management.base import BaseCommand > > +from patchwork.notifications import expire_events > from patchwork.notifications import expire_notifications > from patchwork.notifications import send_notifications > > @@ -34,3 +35,4 @@ class Command(BaseCommand): > (recipient.email, error)) > > expire_notifications() > + expire_events() > diff --git a/patchwork/notifications.py b/patchwork/notifications.py > index 88e96628..b604d579 100644 > --- a/patchwork/notifications.py > +++ b/patchwork/notifications.py > @@ -1,5 +1,6 @@ > # Patchwork - automated patch tracking system > # Copyright (C) 2008 Jeremy Kerr <jk@ozlabs.org> > +# Copyright (C) 2017 Stephen Finucane <stephen@that.guru> > # > # This file is part of the Patchwork package. > # > @@ -31,6 +32,7 @@ from django.template.loader import render_to_string > > from patchwork.models import EmailConfirmation > from patchwork.models import EmailOptout > +from patchwork.models import Event > from patchwork.models import PatchChangeNotification > > > @@ -109,8 +111,16 @@ def expire_notifications(): > EmailConfirmation.objects.filter(q).delete() > > # remove inactive users with no pending confirmation > - pending_confs = EmailConfirmation.objects.values('user') > - users = User.objects.filter(is_active=False).exclude(id__in=pending_confs) > + pending_conf = EmailConfirmation.objects.values('user') > + User.objects.filter(is_active=False).exclude(id__in=pending_conf).delete() > > - # delete users > - users.delete() > + > +def expire_events(): > + """Drop excess events. > + > + Events are generated for almost everything and shouldn't be allowed > + to grow unchecked. This task cleans them up. > + """ > + event_validity = datetime.timedelta(days=settings.EVENTS_VALIDITY_DAYS) > + q = Q(date__lt=datetime.datetime.now() - event_validity) > + Event.objects.filter(q).delete() > diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py > index 4b0d5513..30734ece 100644 > --- a/patchwork/settings/base.py > +++ b/patchwork/settings/base.py > @@ -204,9 +204,12 @@ LOGGING = { > # > # Patchwork settings > # > +# https://patchwork.readthedocs.io/en/latest/deployment/configuration/ > > DEFAULT_ITEMS_PER_PAGE = 100 > > +EVENTS_VALIDITY_DAYS = 14 > + > CONFIRMATION_VALIDITY_DAYS = 7 > > NOTIFICATION_DELAY_MINUTES = 10 > diff --git a/releasenotes/notes/automatically-remove-old-events-4ee222aaf4a6ea6c.yaml b/releasenotes/notes/automatically-remove-old-events-4ee222aaf4a6ea6c.yaml > new file mode 100644 > index 00000000..92590e49 > --- /dev/null > +++ b/releasenotes/notes/automatically-remove-old-events-4ee222aaf4a6ea6c.yaml > @@ -0,0 +1,8 @@ > +--- > +features: > + - | > + The ``EVENTS_VALIDITY_DAYS`` configuration option is added. This configures > + the number of days that an event should be retained for. > + - | > + The ``cron`` management command will now remove events older than > + ``EVENTS_VALIDITY_DAYS`` days. > -- > 2.14.3 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On Thu, 2018-01-25 at 16:21 +1100, Daniel Axtens wrote: > Hi Stephen, > > This is a good idea and I'm happy for it to go in as-is, before the > rest > of the series. > > Reviewed-by: Daniel Axtens <dja@axtens.net> I've been thinking a little more on this and I don't know if I actually want to do this. If we manage to solve the performance impact, we can use events as an auditor and display things like state changes on the patch detail page. I might leave this one for another while. Would still appreciate reviews on the rest of this series though. Stephen > > Regards, > Daniel
Stephen Finucane <stephen@that.guru> writes: > On Thu, 2018-01-25 at 16:21 +1100, Daniel Axtens wrote: >> Hi Stephen, >> >> This is a good idea and I'm happy for it to go in as-is, before the >> rest >> of the series. >> >> Reviewed-by: Daniel Axtens <dja@axtens.net> > > I've been thinking a little more on this and I don't know if I actually > want to do this. If we manage to solve the performance impact, we can > use events as an auditor and display things like state changes on the > patch detail page. I might leave this one for another while. Would > still appreciate reviews on the rest of this series though. I think having this configurable would be the best. Perhaps have 0 = disable, 1+ = delete after N days. (Maybe we need to expose a unique ID for patches to help people who use them.) Regards, Daniel > > Stephen > >> >> Regards, >> Daniel
Daniel Axtens <dja@axtens.net> writes: > Stephen Finucane <stephen@that.guru> writes: > >> On Thu, 2018-01-25 at 16:21 +1100, Daniel Axtens wrote: >>> Hi Stephen, >>> >>> This is a good idea and I'm happy for it to go in as-is, before the >>> rest >>> of the series. >>> >>> Reviewed-by: Daniel Axtens <dja@axtens.net> >> >> I've been thinking a little more on this and I don't know if I actually >> want to do this. If we manage to solve the performance impact, we can >> use events as an auditor and display things like state changes on the >> patch detail page. I might leave this one for another while. Would >> still appreciate reviews on the rest of this series though. > > I think having this configurable would be the best. Perhaps have > 0 = disable, 1+ = delete after N days. > > (Maybe we need to expose a unique ID for patches to help people who use > them.) As it turns out, we already do this. > > Regards, > Daniel > >> >> Stephen >> >>> >>> Regards, >>> Daniel
diff --git a/docs/deployment/configuration.rst b/docs/deployment/configuration.rst index 34748563..3494bd39 100644 --- a/docs/deployment/configuration.rst +++ b/docs/deployment/configuration.rst @@ -51,6 +51,13 @@ This is customizable on a per-user basis from the user configuration page. This option was previously named ``DEFAULT_PATCHES_PER_PAGE``. It was renamed as cover letters are now supported also. +``EVENT_VALIDITY_DAYS`` +~~~~~~~~~~~~~~~~~~~~~~~ + +The number of days to retain :ref:`events` for. After this interval, the +:ref:`cron management command <deployment-final-steps>` will delete the +request. + ``CONFIRMATION_VALIDITY_DAYS`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/usage/overview.rst b/docs/usage/overview.rst index 802de2f1..7ac265f7 100644 --- a/docs/usage/overview.rst +++ b/docs/usage/overview.rst @@ -207,6 +207,8 @@ To-do Lists Patchwork users can store a to-do list of patches. +.. _events: + Events ------ diff --git a/patchwork/management/commands/cron.py b/patchwork/management/commands/cron.py index 2be234b1..653fb0b3 100644 --- a/patchwork/management/commands/cron.py +++ b/patchwork/management/commands/cron.py @@ -19,6 +19,7 @@ from django.core.management.base import BaseCommand +from patchwork.notifications import expire_events from patchwork.notifications import expire_notifications from patchwork.notifications import send_notifications @@ -34,3 +35,4 @@ class Command(BaseCommand): (recipient.email, error)) expire_notifications() + expire_events() diff --git a/patchwork/notifications.py b/patchwork/notifications.py index 88e96628..b604d579 100644 --- a/patchwork/notifications.py +++ b/patchwork/notifications.py @@ -1,5 +1,6 @@ # Patchwork - automated patch tracking system # Copyright (C) 2008 Jeremy Kerr <jk@ozlabs.org> +# Copyright (C) 2017 Stephen Finucane <stephen@that.guru> # # This file is part of the Patchwork package. # @@ -31,6 +32,7 @@ from django.template.loader import render_to_string from patchwork.models import EmailConfirmation from patchwork.models import EmailOptout +from patchwork.models import Event from patchwork.models import PatchChangeNotification @@ -109,8 +111,16 @@ def expire_notifications(): EmailConfirmation.objects.filter(q).delete() # remove inactive users with no pending confirmation - pending_confs = EmailConfirmation.objects.values('user') - users = User.objects.filter(is_active=False).exclude(id__in=pending_confs) + pending_conf = EmailConfirmation.objects.values('user') + User.objects.filter(is_active=False).exclude(id__in=pending_conf).delete() - # delete users - users.delete() + +def expire_events(): + """Drop excess events. + + Events are generated for almost everything and shouldn't be allowed + to grow unchecked. This task cleans them up. + """ + event_validity = datetime.timedelta(days=settings.EVENTS_VALIDITY_DAYS) + q = Q(date__lt=datetime.datetime.now() - event_validity) + Event.objects.filter(q).delete() diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py index 4b0d5513..30734ece 100644 --- a/patchwork/settings/base.py +++ b/patchwork/settings/base.py @@ -204,9 +204,12 @@ LOGGING = { # # Patchwork settings # +# https://patchwork.readthedocs.io/en/latest/deployment/configuration/ DEFAULT_ITEMS_PER_PAGE = 100 +EVENTS_VALIDITY_DAYS = 14 + CONFIRMATION_VALIDITY_DAYS = 7 NOTIFICATION_DELAY_MINUTES = 10 diff --git a/releasenotes/notes/automatically-remove-old-events-4ee222aaf4a6ea6c.yaml b/releasenotes/notes/automatically-remove-old-events-4ee222aaf4a6ea6c.yaml new file mode 100644 index 00000000..92590e49 --- /dev/null +++ b/releasenotes/notes/automatically-remove-old-events-4ee222aaf4a6ea6c.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + The ``EVENTS_VALIDITY_DAYS`` configuration option is added. This configures + the number of days that an event should be retained for. + - | + The ``cron`` management command will now remove events older than + ``EVENTS_VALIDITY_DAYS`` days.
Events are created for most state changes in Patchwork which means they're fairly numerous. Start removing them after a (configurable) interval. Signed-off-by: Stephen Finucane <stephen@that.guru> --- docs/deployment/configuration.rst | 7 +++++++ docs/usage/overview.rst | 2 ++ patchwork/management/commands/cron.py | 2 ++ patchwork/notifications.py | 18 ++++++++++++++---- patchwork/settings/base.py | 3 +++ ...tomatically-remove-old-events-4ee222aaf4a6ea6c.yaml | 8 ++++++++ 6 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/automatically-remove-old-events-4ee222aaf4a6ea6c.yaml