Re: patchwork.ozlabs.org down, and e-mails not recorded
>> socket.error: [Errno 111] Connection refused > > This looks like what would happen if you tried to use pwclient when our > web server was down. This happens every now and then for very short > periods of time when we do updates to various things (including > libraries that the web server itself depends on). > > Do the developers: maybe it is worth catching this error in particular > and giving some reasonable message (or, optionally,trying again after a > short wait - or both). Yep. So I want to rewrite pwclient to use the new API - this will mean we can drop XMLRPC and also help to fix the issue where a patch with ^L in it will break pwclient. When I do that, I will fix this. I have opened https://github.com/getpatchwork/patchwork/issues/172 to help me remember. Regards, Daniel > -- > Cheers, > Stephen Rothwell ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] Fix slow Patch counting query
I have fixed up a small error where I didn't update a test and applied this to master. I'm just waiting for Travis CI to run on my repository, and then I will push it to the main repo. As this is very important to OzLabs, I will spin a 2.1-rc1 soon. Regards, Daniel Daniel Axtens writes: > Stephen Rothwell noticed (way back in September - sorry Stephen!) that > the following query is really slow on OzLabs: > > SELECT COUNT(*) AS "__count" FROM "patchwork_patch" > INNER JOIN "patchwork_submission" ON > ("patchwork_patch"."submission_ptr_id" = "patchwork_submission"."id") > WHERE ("patchwork_submission"."project_id" = 14 AND >"patchwork_patch"."state_id" IN > (SELECT U0."id" AS Col1 FROM "patchwork_state" U0 > WHERE U0."action_required" = true > ORDER BY U0."ordering" ASC)); > > I think this is really slow because we have to join the patch and > submission table to get the project id, which we need to filter the > patches. > > Duplicate the project id in the patch table itself, which allows us to > avoid the JOIN. > > The new query reads as: > SELECT COUNT(*) AS "__count" FROM "patchwork_patch" > WHERE ("patchwork_patch"."patch_project_id" = 1 AND >"patchwork_patch"."state_id" IN > (SELECT U0."id" AS Col1 FROM "patchwork_state" U0 > WHERE U0."action_required" = true > ORDER BY U0."ordering" ASC)); > > Very simple testing on a small, artifical Postgres instance (3 > projects, 102711 patches), shows speed gains of ~1.5-5x for this > query. Looking at Postgres' cost estimates (EXPLAIN) of the first > query vs the second query, we see a ~1.75x improvement there too. > > I suspect the gains will be bigger on OzLabs. > > (It turns out all of this is all for the "| NN patches" counter we > added to the filter bar!!) > > Reported-by: Stephen Rothwell > Signed-off-by: Daniel Axtens > > --- > > This requires a migration, so I don't think we can feasibly do it as a > stable update. > > I think we drop the patch counter for stable and try to get this and > the event stuff merged to master promptly, and just tag 2.1. (To that > end, I will re-read and finish reviewing the event stuff soon.) > --- > patchwork/migrations/0024_patch_patch_project.py | 39 > > patchwork/models.py | 4 +++ > patchwork/parser.py | 1 + > patchwork/views/__init__.py | 2 +- > 4 files changed, 45 insertions(+), 1 deletion(-) > create mode 100644 patchwork/migrations/0024_patch_patch_project.py > > diff --git a/patchwork/migrations/0024_patch_patch_project.py > b/patchwork/migrations/0024_patch_patch_project.py > new file mode 100644 > index ..76d8f144c9dd > --- /dev/null > +++ b/patchwork/migrations/0024_patch_patch_project.py > @@ -0,0 +1,39 @@ > +# -*- coding: utf-8 -*- > +# Generated by Django 1.11.10 on 2018-03-08 01:51 > +from __future__ import unicode_literals > + > +from django.db import migrations, models > +import django.db.models.deletion > + > + > +class Migration(migrations.Migration): > +# per migration 16, but note this seems to be going away > +# in new PostgreSQLs > (https://stackoverflow.com/questions/12838111/south-cannot-alter-table-because-it-has-pending-trigger-events#comment44629663_12838113) > +atomic = False > + > +dependencies = [ > +('patchwork', '0023_timezone_unify'), > +] > + > +operations = [ > +migrations.AddField( > +model_name='patch', > +name='patch_project', > +field=models.ForeignKey(blank=True, null=True, > on_delete=django.db.models.deletion.CASCADE, to='patchwork.Project'), > +preserve_default=False, > +), > + > +# as with 10, this will break if you use non-default table names > +migrations.RunSQL('''UPDATE patchwork_patch SET patch_project_id = > + (SELECT project_id FROM patchwork_submission > +WHERE patchwork_submission.id = > + > patchwork_patch.submission_ptr_id);''' > +), > + > +migrations.AlterField( > +model_name='patch', > +name='patch_project', > + > field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, > to='patchwork.Project'), > +), > + > +] > diff --git a/patchwork/models.py b/patchwork/models.py > index b2491752f04a..3b905c4cd75b 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -423,6 +423,10 @@ class Patch(SeriesMixin, Submission): > archived = models.BooleanField(default=False) > hash = HashField(null=True, blank=True) > > +# duplicate project from submission in subclass so we can count the > +# patches in a project without needing to do a JOIN. > +patch_project = models.ForeignKey(Project, on_delete=mode
Re: [PATCH] Add validation for regular expressions
vkaba...@redhat.com writes: > From: Veronika Kabatova > > Make sure entered regexes compile before saving them. Applied, thank you. Regards, Daniel > > Signed-off-by: Veronika Kabatova > --- > Daniel, feel free to add your Reported-by. > --- > patchwork/models.py | 18 ++ > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/patchwork/models.py b/patchwork/models.py > index b249175..ff1d7dc 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -29,6 +29,7 @@ import re > import django > from django.conf import settings > from django.contrib.auth.models import User > +from django.core.exceptions import ValidationError > from django.db import models > from django.utils.encoding import python_2_unicode_compatible > from django.utils.functional import cached_property > @@ -42,6 +43,13 @@ if settings.ENABLE_REST_API: > from rest_framework.authtoken.models import Token > > > +def validate_regex_compiles(regex_string): > +try: > +re.compile(regex_string) > +except Exception: > +raise ValidationError('Invalid regular expression entered!') > + > + > @python_2_unicode_compatible > class Person(models.Model): > # properties > @@ -74,7 +82,8 @@ class Project(models.Model): > listid = models.CharField(max_length=255) > listemail = models.CharField(max_length=200) > subject_match = models.CharField( > -max_length=64, blank=True, default='', help_text='Regex to match the > ' > +max_length=64, blank=True, default='', > +validators=[validate_regex_compiles], help_text='Regex to match the ' > 'subject against if only part of emails sent to the list belongs to ' > 'this project. Will be used with IGNORECASE and MULTILINE flags. If ' > 'rules for more projects match the first one returned from DB is ' > @@ -232,9 +241,10 @@ class State(models.Model): > class Tag(models.Model): > name = models.CharField(max_length=20) > pattern = models.CharField( > -max_length=50, help_text='A simple regex to match the tag in the' > -' content of a message. Will be used with MULTILINE and IGNORECASE' > -' flags. eg. ^Acked-by:') > +max_length=50, validators=[validate_regex_compiles], > +help_text='A simple regex to match the tag in the content of a ' > +'message. Will be used with MULTILINE and IGNORECASE flags. eg. ' > +'^Acked-by:') > abbrev = models.CharField( > max_length=2, unique=True, help_text='Short (one-or-two letter)' > ' abbreviation for the tag, used in table column headers') > -- > 2.13.6 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: patchwork.ozlabs.org down, and e-mails not recorded
Hi Jeff, On Tue, 20 Mar 2018 15:28:46 -0700 Jeff Kirsher wrote: > > Thanks and so far it looks like it is fixed, I will continue to > monitor it though. I did notice pwclient throwing some errors, such > as: > > Traceback (most recent call last): > File "/home/jtkirshe/bin/pwclient", line 827, in > main() > File "/home/jtkirshe/bin/pwclient", line 780, in main > action_get(rpc, patch_id) > File "/home/jtkirshe/bin/pwclient", line 300, in action_get > patch = rpc.patch_get(patch_id) > File "/usr/lib64/python2.7/xmlrpclib.py", line 1243, in __call__ > return self.__send(self.__name, args) > File "/usr/lib64/python2.7/xmlrpclib.py", line 1602, in __request > verbose=self.__verbose > File "/usr/lib64/python2.7/xmlrpclib.py", line 1283, in request > return self.single_request(host, handler, request_body, verbose) > File "/usr/lib64/python2.7/xmlrpclib.py", line 1311, in single_request > self.send_content(h, request_body) > File "/usr/lib64/python2.7/xmlrpclib.py", line 1459, in send_content > connection.endheaders(request_body) > File "/usr/lib64/python2.7/httplib.py", line 1038, in endheaders > self._send_output(message_body) > File "/usr/lib64/python2.7/httplib.py", line 882, in _send_output > self.send(msg) > File "/usr/lib64/python2.7/httplib.py", line 844, in send > self.connect() > File "/usr/lib64/python2.7/httplib.py", line 821, in connect > self.timeout, self.source_address) > File "/usr/lib64/python2.7/socket.py", line 575, in create_connection > raise err > socket.error: [Errno 111] Connection refused This looks like what would happen if you tried to use pwclient when our web server was down. This happens every now and then for very short periods of time when we do updates to various things (including libraries that the web server itself depends on). Do the developers: maybe it is worth catching this error in particular and giving some reasonable message (or, optionally,trying again after a short wait - or both). -- Cheers, Stephen Rothwell pgpHhR5yOOflo.pgp Description: OpenPGP digital signature ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH] docker: set timezone to Australia/Canberra
The tzinfo package isn't installed in docker, which makes the default timezone UTC. This is unfortunate: the Django TZ in settings/base.py is Australia/Canberra, and having a non-UTC TZ is good for exposing faulty assumptions about what is and isn't UTC. Signed-off-by: Daniel Axtens --- tools/docker/Dockerfile | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index eb6d35d82528..31b2aaeedf49 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -1,6 +1,7 @@ FROM ubuntu:17.10 ARG UID=1000 +ARG TZ="Australia/Canberra" ENV PROJECT_HOME /home/patchwork/patchwork @@ -22,12 +23,15 @@ RUN apt-get update -qq && \ python3.5-dev python3-pip python3-setuptools python3-wheel \ python3.4-dev findutils=4.4.2-7 python3.6-dev \ libmysqlclient-dev mysql-client curl unzip xvfb chromium-chromedriver \ -chromium-browser build-essential git postgresql-client && \ +chromium-browser build-essential git postgresql-client tzdata && \ ln -s /usr/lib/chromium-browser/chromedriver /usr/bin/ # User RUN useradd --uid=$UID --create-home patchwork +# Timezone +RUN rm /etc/localtime; ln -s /usr/share/zoneinfo/$TZ /etc/localtime + # Python requirements. # If you update requirements, you should rebuild the container. # entrypoint.sh will prompt you to do this. -- 2.14.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: patchwork.ozlabs.org down, and e-mails not recorded
On Tue, Mar 20, 2018 at 5:19 AM, Jeremy Kerr wrote: > Hi all, > > Good news: with the logging we had in place recently (and a fix to > correct error reporting), it looks like we've found the problem with > parsing emails from a series. This issue was recently addressed by > Daniel, so I've updated patchwork.ozlabs.org to run the current > stable/2.0 branch, which has fixes for these. Hopefully this resolves > the issue with dropped patches. > > The update should also make the mail parser a little more robust against > transient failures too. > > Let me know how things go over the next few days. > > Thomas and Dave: please keep using the custom addresses you've > configured for the incoming mail feed; this will allow me to keep an eye > on things on my side too. Assuming everything goes fine, we can revert > to the usual address next week. > Thanks and so far it looks like it is fixed, I will continue to monitor it though. I did notice pwclient throwing some errors, such as: Traceback (most recent call last): File "/home/jtkirshe/bin/pwclient", line 827, in main() File "/home/jtkirshe/bin/pwclient", line 780, in main action_get(rpc, patch_id) File "/home/jtkirshe/bin/pwclient", line 300, in action_get patch = rpc.patch_get(patch_id) File "/usr/lib64/python2.7/xmlrpclib.py", line 1243, in __call__ return self.__send(self.__name, args) File "/usr/lib64/python2.7/xmlrpclib.py", line 1602, in __request verbose=self.__verbose File "/usr/lib64/python2.7/xmlrpclib.py", line 1283, in request return self.single_request(host, handler, request_body, verbose) File "/usr/lib64/python2.7/xmlrpclib.py", line 1311, in single_request self.send_content(h, request_body) File "/usr/lib64/python2.7/xmlrpclib.py", line 1459, in send_content connection.endheaders(request_body) File "/usr/lib64/python2.7/httplib.py", line 1038, in endheaders self._send_output(message_body) File "/usr/lib64/python2.7/httplib.py", line 882, in _send_output self.send(msg) File "/usr/lib64/python2.7/httplib.py", line 844, in send self.connect() File "/usr/lib64/python2.7/httplib.py", line 821, in connect self.timeout, self.source_address) File "/usr/lib64/python2.7/socket.py", line 575, in create_connection raise err socket.error: [Errno 111] Connection refused So I downloaded a new copy of pwclient (in case it needed to be updated) and the above error has only happened once. Thought I would make you aware of it, just in case it was not expected. -- Cheers, Jeff ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: patchwork.ozlabs.org down, and e-mails not recorded
Hello Jeremy, On Tue, 20 Mar 2018 20:19:01 +0800, Jeremy Kerr wrote: > Good news: with the logging we had in place recently (and a fix to > correct error reporting), it looks like we've found the problem with > parsing emails from a series. This issue was recently addressed by > Daniel, so I've updated patchwork.ozlabs.org to run the current > stable/2.0 branch, which has fixes for these. Hopefully this resolves > the issue with dropped patches. > > The update should also make the mail parser a little more robust against > transient failures too. > > Let me know how things go over the next few days. > > Thomas and Dave: please keep using the custom addresses you've > configured for the incoming mail feed; this will allow me to keep an eye > on things on my side too. Assuming everything goes fine, we can revert > to the usual address next week. Thanks a lot for this debugging effort! We'll monitor how patchwork behaves in the next days and give you some feedback (good or bad). Thanks again! Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: patchwork.ozlabs.org down, and e-mails not recorded
Hi all, Good news: with the logging we had in place recently (and a fix to correct error reporting), it looks like we've found the problem with parsing emails from a series. This issue was recently addressed by Daniel, so I've updated patchwork.ozlabs.org to run the current stable/2.0 branch, which has fixes for these. Hopefully this resolves the issue with dropped patches. The update should also make the mail parser a little more robust against transient failures too. Let me know how things go over the next few days. Thomas and Dave: please keep using the custom addresses you've configured for the incoming mail feed; this will allow me to keep an eye on things on my side too. Assuming everything goes fine, we can revert to the usual address next week. Cheers, Jeremy ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH] parsemail: Clarify exit codes
jk reports that the patchwork error codes are really unhelpful for correct integration with an MDA. In particular they make sorting out failures into a separate queue very difficult. Make this better and clearer. Update the comment for parse_mail regarding return values and exceptions to line up with how the function actually works and how we use the results. Reported-by: Jeremy Kerr Fixes: #171 Signed-off-by: Daniel Axtens --- patchwork/management/commands/parsemail.py | 16 patchwork/parser.py| 9 - 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/patchwork/management/commands/parsemail.py b/patchwork/management/commands/parsemail.py index 52ec8bc56899..1ed12ae886cf 100644 --- a/patchwork/management/commands/parsemail.py +++ b/patchwork/management/commands/parsemail.py @@ -77,12 +77,20 @@ class Command(base.BaseCommand): logger.warning("Broken email ignored") return +# it's important to get exit codes correct here. The key is to allow +# proper separation of real errors vs expected 'failures'. +# +# patch/comment parsed:0 +# no parseable content found: 0 +# db integrity/other db error: 1 (this will mean dups are logged, which +# might get annoying - we'll see) +# broken email (ValueError): 1 (this could also be noisy, if an issue +# we could use a different return code) try: result = parse_mail(mail, options['list_id']) -if result: -sys.exit(0) -logger.warning('Failed to parse mail') -sys.exit(1) +if result is None: +logger.warning('Nothing added to database') except Exception: logger.exception('Error when parsing incoming email', extra={'mail': mail.as_string()}) +sys.exit(1) diff --git a/patchwork/parser.py b/patchwork/parser.py index c1f40a577fa3..019ca20bb159 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -922,7 +922,14 @@ def parse_mail(mail, list_id=None): list_id (str): Mailing list ID Returns: -None +patch/cover letter/comment +Or None if nothing is found in the mail + or X-P-H: ignore + or project not found + +Raises: +ValueError if there is an error in parsing +Some db errors are passed through (e.g. IntegrityError) """ # some basic sanity checks if 'From' not in mail: -- 2.14.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork