Re: patchwork.ozlabs.org down, and e-mails not recorded

2018-03-20 Thread Daniel Axtens
>> 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

2018-03-20 Thread Daniel Axtens
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

2018-03-20 Thread Daniel Axtens

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

2018-03-20 Thread Stephen Rothwell
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

2018-03-20 Thread Daniel Axtens
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

2018-03-20 Thread Jeff Kirsher
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

2018-03-20 Thread Thomas Petazzoni
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

2018-03-20 Thread Jeremy Kerr

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

2018-03-20 Thread Daniel Axtens
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