[RFC PATCH] docker: Add support for using eatmydata in the database

[Russell Currey]

When running tox on a VM with presumably pretty busy spinning disks,
using eatmydata with the database took running one configuration's test
suite from (no exaggeration) 20 minutes down to 60 seconds.

It makes a huge difference to test speed, so we should make it easily
available for developers.  The primary motivation here was to
automatically test each patch in a timeframe that isn't insane.

Open to ideas on how to organise this, whether we do it for MySQL too
(which we probably should), whether the base directory should have these
files in it, what to call the Dockerfile, etc.  I think it's a good
thing to have in the repo, though.

Signed-off-by: Russell Currey 
---
 docker-compose-eatmydata.yml  | 32 +++
 tools/docker/Dockerfile.eatmydata |  9 +
 2 files changed, 41 insertions(+)
 create mode 100644 docker-compose-eatmydata.yml
 create mode 100644 tools/docker/Dockerfile.eatmydata

diff --git a/docker-compose-eatmydata.yml b/docker-compose-eatmydata.yml
new file mode 100644
index 000..27d1604
--- /dev/null
+++ b/docker-compose-eatmydata.yml
@@ -0,0 +1,32 @@
+version: "3"
+services:
+  db:
+build:
+  context: .
+  dockerfile: ./tools/docker/Dockerfile.eatmydata
+volumes:
+  - ./tools/docker/db/postdata:/var/lib/postgresql/data
+environment:
+  - POSTGRES_PASSWORD=password
+
+  web:
+build:
+  context: .
+  dockerfile: ./tools/docker/Dockerfile
+  args:
+- UID
+depends_on:
+  - db
+command: python3 manage.py runserver 0.0.0.0:8000
+volumes:
+  - .:/home/patchwork/patchwork/
+ports:
+  - "8000:8000"
+environment:
+  - UID
+  - PW_TEST_DB_HOST=db
+  - PW_TEST_DB_PORT=5432
+  - PW_TEST_DB_TYPE=postgres
+  - PW_TEST_DB_USER=postgres
+  - PW_TEST_DB_PASS=password
+  - PGPASSWORD=password
diff --git a/tools/docker/Dockerfile.eatmydata 
b/tools/docker/Dockerfile.eatmydata
new file mode 100644
index 000..693cbb3
--- /dev/null
+++ b/tools/docker/Dockerfile.eatmydata
@@ -0,0 +1,9 @@
+FROM postgres:9.6
+
+RUN apt-get update \
+ && apt-get install -y eatmydata \
+ && apt-get autoremove -y \
+ && rm -rf /var/lib/apt/lists/*
+
+ENTRYPOINT [ "/usr/bin/eatmydata", "/usr/local/bin/docker-entrypoint.sh" ]
+CMD ["postgres"]
-- 
2.21.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH] docker: Install libpq-dev to fix psycopg2-binary build

[Russell Currey]

On Wed, 2019-05-01 at 17:27 +1000, Andrew Donnellan wrote:
> On 1/5/19 2:35 pm, Russell Currey wrote:
> > psycopg2-binary fails if pg_config isn't installed, which is
> > provided by
> > libpq-dev.
> > 
> > This seems strange to me since psycopg2-binary suggests that
> > you use psycopg2-binary instead (of itself) if you don't want to
> > build
> > psycopg2 so you wouldn't need pg_config, which is very confusing.
> > 
> > It's possible that psycopg2-binary only needs to compile itself on
> > non-x86 platforms, since I hit this on ppc64le.
> > 
> > Anyway, it works when this is added.
> 
> I don't think there is a binary psycopg2-binary build for anything
> other 
> than i686/x86_64?

Right, so it must fall back and build the thing if it doesn't exist for
the platform.

> 
> Reviewed-by: Andrew Donnellan 
> 
> > Signed-off-by: Russell Currey 
> > ---
> >   tools/docker/Dockerfile | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
> > index b9ecdb5..eef40e4 100644
> > --- a/tools/docker/Dockerfile
> > +++ b/tools/docker/Dockerfile
> > @@ -26,7 +26,7 @@ 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 build-essential \
> > -git postgresql-client tzdata
> > +git postgresql-client tzdata libpq-dev
> >   
> >   # User
> >   RUN useradd --uid=$UID --create-home patchwork
> > 

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 1/2] docker: Use Ubuntu ports repositories on non-x86 architectures

[Russell Currey]

This should allow Patchwork to run "out of the box" in Docker on any
architecture with a) an Ubuntu port and b) support in the Postgres
multiarch Docker image, which includes at least arm64 and ppc64le.

It's a little gross hacking the Dockerfile like this, but I'm not sure
there's a more elegant way to do it.  Unfortunately it doesn't seem like
there's any way to do conditional COPY, and anything in RUN is plain
/bin/sh, so that's why it looks like it does.

Tested on ppc64le and on x86_64.

Signed-off-by: Russell Currey 
---
 tools/docker/Dockerfile| 14 --
 tools/docker/trusty-ports.list |  3 +++
 tools/docker/xenial-ports.list |  3 +++
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 tools/docker/trusty-ports.list
 create mode 100644 tools/docker/xenial-ports.list

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index eef40e4..76bb6b2 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -15,11 +15,21 @@ ENV DJANGO_SETTINGS_MODULE patchwork.settings.dev
 ENV DEBIAN_FRONTEND noninteractive
 ENV PYTHONUNBUFFERED 1
 
+
 # System
 # trusty and findutils is for python3.4; xenial is for python3.5
 # TODO(stephenfin): Are curl, unzip required?
-COPY tools/docker/trusty.list /etc/apt/sources.list.d/trusty.list
-COPY tools/docker/xenial.list /etc/apt/sources.list.d/xenial.list
+COPY tools/docker/*.list /etc/apt/sources.list.d/
+
+RUN cd /etc/apt/sources.list.d; \
+echo $(uname -m) > /tmp/arch; \
+if [ $(cat /tmp/arch) != 'x86_64' ] && grep -q -v "i.86" /tmp/arch; then \
+mv trusty-ports.list trusty.list; \
+mv xenial-ports.list xenial.list; \
+else \
+rm *-ports.list; \
+fi
+
 RUN apt-get update -qq && \
 apt-get install -y --no-install-recommends --allow-downgrades \
 python-dev python-pip python-setuptools python-wheel \
diff --git a/tools/docker/trusty-ports.list b/tools/docker/trusty-ports.list
new file mode 100644
index 000..ebcf4fa
--- /dev/null
+++ b/tools/docker/trusty-ports.list
@@ -0,0 +1,3 @@
+deb http://ports.ubuntu.com/ubuntu-ports/ trusty main
+deb http://ports.ubuntu.com/ubuntu-ports/ trusty-updates main
+deb http://ports.ubuntu.com/ubuntu-ports/ trusty-security main
diff --git a/tools/docker/xenial-ports.list b/tools/docker/xenial-ports.list
new file mode 100644
index 000..d84641f
--- /dev/null
+++ b/tools/docker/xenial-ports.list
@@ -0,0 +1,3 @@
+deb http://ports.ubuntu.com/ubuntu-ports/ xenial main
+deb http://ports.ubuntu.com/ubuntu-ports/ xenial-updates main
+deb http://ports.ubuntu.com/ubuntu-ports/ xenial-security main
-- 
2.21.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 2/2] docs: Mention Postgres for Docker development install

[Russell Currey]

Might as well since it's there, and it gives some clue to anyone trying
to use Docker on non-x86.  I figured it was best to leave this out of
the README since it's incredibly niche.

Signed-off-by: Russell Currey 
---
 docs/development/installation.rst | 4 
 1 file changed, 4 insertions(+)

diff --git a/docs/development/installation.rst 
b/docs/development/installation.rst
index 52dc2c3..0ab755f 100644
--- a/docs/development/installation.rst
+++ b/docs/development/installation.rst
@@ -37,6 +37,10 @@ configure Patchwork using Docker:
 
   $ docker-compose build
 
+   To use Postgres instead of MySQL, give the ``-f docker-compose-pg.yml``
+   argument to ``docker-compose``.  This is required on non-x86 architectures
+   as the MySQL Docker images do not have multiarch support.
+
 #. Run ``docker-compose up``:
 
.. code-block:: shell
-- 
2.21.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH] docker: Install libpq-dev to fix psycopg2-binary build

[Russell Currey]

psycopg2-binary fails if pg_config isn't installed, which is provided by
libpq-dev.

This seems strange to me since psycopg2-binary suggests that
you use psycopg2-binary instead (of itself) if you don't want to build
psycopg2 so you wouldn't need pg_config, which is very confusing.

It's possible that psycopg2-binary only needs to compile itself on
non-x86 platforms, since I hit this on ppc64le.

Anyway, it works when this is added.

Signed-off-by: Russell Currey 
---
 tools/docker/Dockerfile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index b9ecdb5..eef40e4 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -26,7 +26,7 @@ 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 build-essential \
-git postgresql-client tzdata
+git postgresql-client tzdata libpq-dev
 
 # User
 RUN useradd --uid=$UID --create-home patchwork
-- 
2.21.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH] README: add .env file to installation instructions

[Russell Currey]

Creating the .env file is mentioned in the installation documentation
but not in the README, so following only the steps mentioned there will
fail.  Add this and add a `cd patchwork` in there for good measure so
you could straight up copy paste the steps.

Signed-off-by: Russell Currey 
---
 README.rst | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/README.rst b/README.rst
index 38c1847..b45c3e6 100644
--- a/README.rst
+++ b/README.rst
@@ -61,11 +61,16 @@ environment. To install Patchwork:
 
$ git clone https://github.com/getpatchwork/patchwork.git
 
-3. Build the images. This will download over 200MB from the internet::
+3. Create a ``.env`` file in the root directory of the project and store your
+   ``UID`` attribute there::
+
+   $ cd patchwork && echo "UID=$UID" > .env
+
+4. Build the images. This will download over 200MB from the internet::
 
$ docker-compose build
 
-4. Run `docker-compose up`::
+5. Run `docker-compose up`::
 
$ docker-compose up
 
-- 
2.21.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: How to re-send a series of patches?

[Russell Currey]

On Thu, 2018-11-08 at 14:28 +, Stephen Finucane wrote:
> On Fri, 2018-11-09 at 00:42 +1100, Daniel Axtens wrote:
> > Hi Christopher,
> > 
> > > What I'm really trying to is get to the point where the series
> > > can be
> > > tested. I know you've written and talked about using Patchwork
> > > for this
> > > [1].
> > 
> > You might want to check out snowpatch [1], it does this sort of
> > thing at
> > the moment and is live on linuxppc-dev mailing list. For example
> > here's
> > some links of it running tests against patches and series sent to
> > the
> > list:
> > 
> >  - https://patchwork.ozlabs.org/patch/994798/ (early patch in
> > series, 2
> >tests run - does it apply and does it pass style-checks)
> >
> >  - https://patchwork.ozlabs.org/patch/994810/ (final patch in
> > series, 6
> >tests run including 4 build-tests)
> > 
> >  - https://patchwork.ozlabs.org/patch/993734/ (standalone patch, 6
> > tests run)
> > 
> > AIUI, it's a very similar concept to Stephen's blog post (pw +
> > jenkins),
> > but it's all wrapped up nicely for you.
> 
> I was going to suggest this but after looking through the code I saw
> references to concepts from thefreedesktop-fork like TestResult
> (called
> Checks in upstream) and series revisions [1]. Does this definitely
> work
> against upstream, yeah?

Hey Stephen, yeah it only works against upstream now.  I originally
wrote it years ago against the fork since upstream patchwork didn't
have the necessary features at that point.  Once that changed, I
rewrote it to work with the upstream API, but some of the old terms
still remain.  I'll add that to the list of things to do.

- Russell

> 
> Stephen
> 
> [1] 
> https://github.com/ruscur/snowpatch/blob/8e93b453ad/src/patchwork.rs#L176-L204
> 
> > Regards,
> > Daniel
> > 
> > [1]: https://github.com/ruscur/snowpatch
> 
> ___
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: Continuous integration with buildbot

[Russell Currey]

On Fri, 2017-10-13 at 13:01 +1100, Daniel Axtens wrote:
> Hi David,
> 
> You might also want to look at Snowpatch - it's a shim between patchwork
> and Jenkins, so it might have some helpful concepts.
> 
> https://developer.ibm.com/open/openprojects/snowpatch/
> https://github.com/ruscur/snowpatch

We were keen on supporting multiple CI backends (including Buildbot), but Andrew
and I have been far too busy recently to work on it.  It's definitely
technically feasible in any case, and there's some code/ideas you could steal.

- Russell

> 
> Regards,
> Daniel
> 
> > Hello all,
> > 
> > I'm guessing if one of you successfully enabled some linking between
> > patchwork and buildbot [0].
> > 
> > According to the patchwork documentation, checks must be done using the
> > REST API.
> > 
> > Does that mean we must implement a poller in the buildbot that checks
> > for new patches? I have a few concerns:
> > 
> >   1. I need to check how buildbot knows that a patch has already been
> >  tested to avoid rebuilding over and over,
> >   2. I should also check how to make buildbot as secure as possible
> >  since tests can be ran with insecure code sent via mails.
> > 
> > Any advices are welcome,
> > 
> > Regards,
> > 
> > [0]: http://buildbot.net
> > 
> > -- 
> > David Demelier
> > ___
> > Patchwork mailing list
> > Patchwork@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork
> 
> ___
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [RFC PATCH] REST: enable token authentication

[Russell Currey]

On Thu, 2017-05-25 at 22:26 +0100, Stephen Finucane wrote:
> On Thu, 2017-05-25 at 18:47 +1000, Andrew Donnellan wrote:
> > Token authentication is generally viewed as a more secure option for
> > API
> > authentication than storing a username and password.
> > 
> > Django REST Framework gives us a TokenAuthentication class and an
> > authtoken
> > app that we can use to generate random tokens and authenticate to API
> > endpoints.
> > 
> > Enable DRF's token support and add options to the user profile view
> > to view
> > or regenerate tokens.
> > 
> > Signed-off-by: Andrew Donnellan 
> 

FWIW it looks good to me.

> Yup, totally onboard with this, even for 2.0. I'd be in favor of
> removing Basic Auth if we add this. I can take documentation if you
> want to handle the other items. I imagine tests shouldn't be too
> arduous either.

What exactly does BA get used for at the moment?

- Russell

> 
> Think this is something you could wire up in the next few days? :)
> 
> Stephen
> 
> > ---
> > 
> > This is an RFC; I haven't written any tests or documentation, UI's a
> > bit
> > ugly, need to split patches.
> > ---
> >  patchwork/settings/base.py |  6 ++
> >  patchwork/signals.py   | 10 ++
> >  patchwork/templates/patchwork/profile.html | 23
> > +--
> >  patchwork/urls.py  |  4 
> >  patchwork/views/bundle.py  | 12 
> >  patchwork/views/user.py| 19 +++
> >  6 files changed, 68 insertions(+), 6 deletions(-)
> > 
> > diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py
> > index 26c75c9..6fd98a7 100644
> > --- a/patchwork/settings/base.py
> > +++ b/patchwork/settings/base.py
> > @@ -143,6 +143,7 @@ try:
> >  
> >  INSTALLED_APPS += [
> >  'rest_framework',
> > +'rest_framework.authtoken',
> >  'django_filters',
> >  ]
> >  except ImportError:
> > @@ -158,6 +159,11 @@ REST_FRAMEWORK = {
> >  'rest_framework.filters.SearchFilter',
> >  'rest_framework.filters.OrderingFilter',
> >  ),
> > +'DEFAULT_AUTHENTICATION_CLASSES': (
> > +'rest_framework.authentication.SessionAuthentication',
> > +'rest_framework.authentication.BasicAuthentication',
> > +'rest_framework.authentication.TokenAuthentication',
> > +),
> >  'SEARCH_PARAM': 'q',
> >  'ORDERING_PARAM': 'order',
> >  }
> > diff --git a/patchwork/signals.py b/patchwork/signals.py
> > index 208685c..f335525 100644
> > --- a/patchwork/signals.py
> > +++ b/patchwork/signals.py
> > @@ -19,6 +19,7 @@
> >  
> >  from datetime import datetime as dt
> >  
> > +from django.conf import settings
> >  from django.db.models.signals import post_save
> >  from django.db.models.signals import pre_save
> >  from django.dispatch import receiver
> > @@ -239,3 +240,12 @@ def create_series_completed_event(sender,
> > instance, created, **kwargs):
> >  
> >  if instance.series.received_all:
> >  create_event(instance.series)
> > +
> > +
> > +if settings.ENABLE_REST_API:
> > +from rest_framework.authtoken.models import Token
> > +@receiver(post_save, sender=settings.AUTH_USER_MODEL)
> > +def create_user_created_event(sender, instance=None,
> > created=False,
> > +  **kwargs):
> > +if created:
> > +Token.objects.create(user=instance)
> > diff --git a/patchwork/templates/patchwork/profile.html
> > b/patchwork/templates/patchwork/profile.html
> > index f976195..c7be044 100644
> > --- a/patchwork/templates/patchwork/profile.html
> > +++ b/patchwork/templates/patchwork/profile.html
> > @@ -133,8 +133,27 @@ address.
> >  
> >  
> >  
> > -Authentication
> > -Change password
> > +  Authentication
> > +  
> > +{% csrf_token %}
> > +
> > +  
> > +   Password:
> > +   Change
> > password
> > +  
> > +  
> > +   API Token:
> > +   
> > + {% if api_token %}
> > + {{ api_token }}
> > + 
> > + {% else %}
> > + 
> > + {% endif %}
> > +   
> > +  
> > +
> > +  
> >  
> >  
> >  
> > diff --git a/patchwork/urls.py b/patchwork/urls.py
> > index 1871c9a..aa49b4d 100644
> > --- a/patchwork/urls.py
> > +++ b/patchwork/urls.py
> > @@ -101,6 +101,10 @@ urlpatterns = [
> >  auth_views.password_reset_complete,
> >  name='password_reset_complete'),
> >  
> > +# token change
> > +url(r'^user/generate-token/$', user_views.generate_token,
> > +name='generate_token'),
> > +
> >  # login/logout
> >  url(r'^user/login/$', auth_views.login,
> >  {'template_name': 'patchwork/login.html'},
> > diff --git a/patchwork/views/bundle.py b/patchwork/views/bundle.py
> > index 387b7c6..bb331f4 100644
> > --- a/patchwork/views/bundle.py
> > +++ b/patchwork/views/bundle.py
> > @@ -36,17 +36,21 @@ from patchwork.views import generic_list
> >  from 

Re: Patchwork 2.0 ready for takeoff?

[Russell Currey]

On Mon, 2017-05-01 at 08:47 +1000, Daniel Axtens wrote:
> Hi Stephen,
> 
> Apologies on my patchwork-silence; I've recently moved from IBM to
> Canonical so I've been a bit flat-out.
> 
> I haven't tested the patches for support for distro-package Xenial, so
> I'd like to do that, and also do some Debian (and maybe CentOS?)
> tests. I am hoping to get that done in the next couple of days.
> 
> I also think we should do an RC after we land all the features; that'll
> give people a bit of warning and an impetus to test (as well as definite
> target to test, rather than the somewhat more nebulous 'master').

I can do a bunch of testing if there's an rc.

- ruscur

> 
> Regards,
> Daniel
> ___
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2] REST: allow fetching of subject prefixes

[Russell Currey]

On Tue, 2017-02-21 at 16:45 +1100, Daniel Axtens wrote:
> Some mailing lists accept patches for multiple projects, and use
> a subject prefix to differentiate the projects.
> 
> Therefore, for snowpatch, it's useful to be able to fetch the
> subject prefixes.
> 
> Export subject prefixes in the REST API.
> 
> Signed-off-by: Daniel Axtens <d...@axtens.net>
> 

Tested-by: Russell Currey <rus...@russell.cc>
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH] REST: allow fetching of subject prefixes (categories)

[Russell Currey]

On Tue, 2017-02-14 at 08:47 +1100, Daniel Axtens wrote:
> Hi Stephen,
> 
> > On Sat, 2017-02-11 at 21:55 +1100, Daniel Axtens wrote:
> > > Some mailing lists accept patches for multiple projects, and use
> > > a subject prefix to differentiate the projects.
> > 
> > A couple of comments on this starting with this one: should we not be
> > considering patches like this as belonging to a different 'Project' in
> > Patchwork? If we don't do already support something, maybe we should?
> > 
> > Could you provide an example of a mailing list that does this?
> 
> Yes - OpenBMC does this. The one mailing list supports the OpenBMC
> project, and within that they deal with code for several different
> components - the kernel, u-boot, etc:
> 
> https://patchwork.ozlabs.org/project/openbmc/list/?state=*
> 
> They (generally!) use these subject prefixes to distinguish between
> patches for different components/projects.
> 
> > > +def get_categories(self, instance):
> > > +return clean_subject(instance.name)[1]
> > 
> > This is going to run for 'page_size' patches on the list operation. I
> > don't know how expensive each run is but seeing as there are regexes
> > involved I doubt it's free. I wonder if we should access this via a
> > 'cached_property' field on 'Submission'?
> 
> That's a good idea. I'll spin a v2 with that.
> 
> > 
> > >  def get_mbox(self, instance):
> > >  request = self.context.get('request')
> > > @@ -100,10 +105,10 @@ class
> > > PatchListSerializer(HyperlinkedModelSerializer):
> > >  fields = ('id', 'url', 'project', 'msgid', 'date', 'name',
> > >    'commit_ref', 'pull_url', 'state', 'archived',
> > > 'hash',
> > >    'submitter', 'delegate', 'mbox', 'series',
> > > 'check', 'checks',
> > > -  'tags')
> > > +  'tags', 'categories')
> > 
> > I'm not sure about 'categories' - this will also return things like the
> > version ('v2') and the number of the patch if it's in a series ('1/5'),
> > correct? Labels might be more valid, if so (though I had planned to use
> > that name for a more significant feature in v2.1 or so [1]).
> 
> That's valid. I'm happy to leave 'labels' if you have designs on that
> for future. How about 'prefixes'? That also works well for leaving in
> version and series markers - they're all prefixes.
> 
> > 
> > >  read_only_fields = ('project', 'msgid', 'date', 'name',
> > > 'hash',
> > >  'submitter', 'mbox', 'mbox', 'series',
> > > 'check',
> > > -'checks', 'tags')
> > > +'checks', 'tags', 'categories')
> > 
> > Do we want to be able to filter on these? I don't know if you can do
> > this for non-model fields though...
> 
> At the moment, no. The use case is our CI system where we want to be
> able to take each patch one by one and figure out which tree to apply it
> to. So we don't need to filter on categories/prefixes.

Even if *we* don't, it still seems like a good idea, i.e. for someone who is
only interested in patches for a specific category.  Having multiple projects on
the same list is dumb, but I've seen it enough times that it'd be good to
somewhat support.

> 
> Regards,
> Daniel
> 
> > Stephen
> > 
> > [1] https://github.com/getpatchwork/patchwork/issues/22

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH] REST: allow fetching of subject prefixes (categories)

[Russell Currey]

On Sat, 2017-02-11 at 21:55 +1100, Daniel Axtens wrote:
> Some mailing lists accept patches for multiple projects, and use
> a subject prefix to differentiate the projects.
> 
> Therefore, for snowpatch, it's useful to be able to fetch the
> subject prefixes. Snowpatch calls these 'categories', so lets
> adopt their terminology.
> 
> Export categeories in the REST API.
> 
> Signed-off-by: Daniel Axtens 
> ---

Yeah, this is very useful for mailing lists that receive patches for multiple
projects. I don't really care what terminology patchwork uses, though.

It might also be useful to have delegation rules based on these categories if
that's not already a thing (I don't know anything about delegation)

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2 12/13] REST: Add '/series' endpoint

[Russell Currey]

On Sat, 2016-11-19 at 16:51 +, Stephen Finucane wrote:
> Adopt a hybrid approach, by adding an additional series endpoint to the
> existing patch endpoint:
> 
> /series/${series_id}/
> /patches/${patch_id}/
> 
> This is based on the approach described here:
> 
> http://softwareengineering.stackexchange.com/a/275007/106804
> 
> This is necessary due to the N:N mapping of series and patches: it's
> possible for a patch to belong to many series, and a series usually
> contains many patches. This means it is not possible to rely on the
> patch endpoint alone.
> 
> It is also necessary to add a cover letter endpoint, such that the
> series body can include this.
> 
> Signed-off-by: Stephen Finucane 
> Cc: Andy Doan 
> 

Currently the url field points to /project// rather than /series//, and
there's no reference to the project that the series belongs to, which would be
nice.

Also to be clear, the patches array is guaranteed to be in order, right?
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [RFC 0/2] Add REST API filtering/querying

[Russell Currey]

On Thu, 2016-11-17 at 01:47 +, Stephen Finucane wrote:
> This requires the series REST API series.

This breaks stuff, I'm not sure why.

Environment:

Request Method: GET
Request URL: http:///project//list/

Django Version: 1.8
Python Version: 2.7.12
Installed Applications:
['django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.humanize',
 'django.contrib.messages',
 'django.contrib.sessions',
 'django.contrib.sites',
 'django.contrib.admin',
 'django.contrib.staticfiles',
 'patchwork',
 'rest_framework',
 'django_filters']
Installed Middleware:
['django.middleware.common.CommonMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.admindocs.middleware.XViewMiddleware']


Traceback:
File "/home/patchwork/.local/lib/python2.7/site-
packages/django/core/handlers/base.py" in get_response
  132. response = wrapped_callback(request, *callback_args,
**callback_kwargs)
File "/home/patchwork//patchwork/patchwork/views/patch.py" in patches
  114.view_args={'project_id': project.linkname})
File "/home/patchwork//patchwork/patchwork/views/__init__.py" in generic_list
  196. filters = Filters(request)
File "/home/patchwork//patchwork/patchwork/filters.py" in __init__
  455. self._filters = [c(self) for c in filterclasses]
File "/home/patchwork//patchwork/patchwork/filters.py" in __init__
  98. super(SeriesFilter, self).__init__(filters)

Exception Type: TypeError at /project//list/
Exception Value: super(type, obj): obj must be an instance or subtype of type

> 
> Stephen Finucane (2):
>   REST: Integrate django-filter support
>   REST: Integrate searching
> 
>  patchwork/api/check.py |  2 ++
>  patchwork/api/cover.py |  3 +++
>  patchwork/api/patch.py |  3 +++
>  patchwork/api/person.py|  2 +-
>  patchwork/api/project.py   |  3 ++-
>  patchwork/api/series.py|  4 +++-
>  patchwork/api/user.py  |  2 +-
>  patchwork/filters.py   | 41 +
>  patchwork/settings/base.py |  8 +++-
>  requirements-test.txt  |  2 +-
>  10 files changed, 64 insertions(+), 6 deletions(-)
> 

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2 3/6] parser: Add series parsing

[Russell Currey]

On Mon, 2016-09-05 at 14:25 +1000, Andrew Donnellan wrote:
> On 05/09/16 14:19, Russell Currey wrote:
> > 
> > In testing v2 I found a weird issue I didn't find before.
> > 
> > If you send a new series in reply to the cover letter of a previous series,
> > it
> > appends the patches to the previous series.  This is rather confusing as you
> > would think any patches sent in reply to a cover letter would belong to that
> > series, but they clearly should be treated differently in some cases, as you
> > can
> > see in the screenshot below:
> > 
> > https://i.imgur.com/8Yi9IjR.png
> 
> Or in reply to patch 1 of the initial series - the one in your 
> screenshot doesn't have a cover letter.

Yeah, you're right, good catch.

> 
> > 
> > Maybe use a reset in numbering?  This would be harder to parse if both
> > series
> > had the same number of patches, I'm imagining something like the following:
> > 
> > - [1/3] patch sent in reply to cover letter, new series (#1)
> > - [2/3] patch sent in reply to cover letter, append to #1
> > - [3/3] patch sent in reply to cover letter, append to #1
> > - [1/3] patch sent in reply to cover letter, new series (#2)
> > - [2/3] patch sent in reply to cover letter:
> > - see that series #2 doesn't have a 2/3
> > - see that the date of the patch means it's probably for series #2
> > - append to series #2
> > - [3/3] is the same as above.
> 
> This is the sanest approach I think. For each 1/N patch, create a new 
> series.
> 

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH v2 3/6] parser: Add series parsing

[Russell Currey]

On Sun, 2016-09-04 at 01:24 +0100, Stephen Finucane wrote:
> From: Stephen Finucane <stephen.finuc...@intel.com>
> 
> It is now possible to parse and store series, so do just that.
> The parsing at the moment is based on both RFC822 headers and
> subject lines.
> 
> Signed-off-by: Stephen Finucane <stephen.finuc...@intel.com>
> Tested-by: Russell Currey <rus...@russell.cc>
> ---

In testing v2 I found a weird issue I didn't find before.

If you send a new series in reply to the cover letter of a previous series, it
appends the patches to the previous series.  This is rather confusing as you
would think any patches sent in reply to a cover letter would belong to that
series, but they clearly should be treated differently in some cases, as you can
see in the screenshot below:

https://i.imgur.com/8Yi9IjR.png

(this is my test instance so it's not publicly accessible)

So it might be tricky to define what *does* deserve a new series in this case,
since:

- can't use author as some patches might be from a different author
- can't use date because what if someone sends patches at 23:59

Maybe use a reset in numbering?  This would be harder to parse if both series
had the same number of patches, I'm imagining something like the following:

- [1/3] patch sent in reply to cover letter, new series (#1)
- [2/3] patch sent in reply to cover letter, append to #1
- [3/3] patch sent in reply to cover letter, append to #1
- [1/3] patch sent in reply to cover letter, new series (#2)
- [2/3] patch sent in reply to cover letter:
- see that series #2 doesn't have a 2/3
- see that the date of the patch means it's probably for series #2
- append to series #2
- [3/3] is the same as above.

That's probably confusing...but hopefully there's a simple solution.  If an
automated system was testing series, applying every patch in the series in the
image above would not be what you wanted.

- Russell
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH 5/7] Allow use of Docker for development

[Russell Currey]

On Tue, 2016-08-02 at 13:44 +1000, Daniel Axtens wrote:
> > 
> > Would it be possible to use chromium instead?  It's probably included in
> > distributions and wouldn't require adding a third-party repo, and worse,
> > proprietary software.
> > 
> That's a fair point; I'll have a look.
> 
> Would you be opposed to switching to firefox? That might be even easier.

Of course, whatever works.

> 
> Regards,
> Daniel
> 
> > 
> > > 
> > > 
> > > Signed-off-by: Daniel Axtens 
> > > ---
> > >  .dockerignore |  3 ++
> > >  README.md | 38 +++-
> > >  docker-compose.yml| 23 ++
> > >  docker/Dockerfile | 50 +++
> > >  docker/bashrc |  5 
> > >  docker/db/.dockerignore   |  1 +
> > >  docker/db/.gitignore  |  1 +
> > >  docker/db/Dockerfile  | 10 +++
> > >  docker/entrypoint.sh  | 76
> > > +++
> > >  docker/google-chrome.list |  1 +
> > >  10 files changed, 207 insertions(+), 1 deletion(-)
> > >  create mode 100644 .dockerignore
> > >  create mode 100644 docker-compose.yml
> > >  create mode 100644 docker/Dockerfile
> > >  create mode 100644 docker/bashrc
> > >  create mode 100644 docker/db/.dockerignore
> > >  create mode 100644 docker/db/.gitignore
> > >  create mode 100644 docker/db/Dockerfile
> > >  create mode 100755 docker/entrypoint.sh
> > >  create mode 100644 docker/google-chrome.list

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH 5/7] Allow use of Docker for development

[Russell Currey]

On Tue, 2016-08-02 at 10:24 +1000, Daniel Axtens wrote:
> This makes it possible to use Docker and docker-compose for development
> as an alternative to Vagrant.
> 
> I quite liked vagrant a couple of years ago, but currently:
> 
>  * Trying to install VirtualBox on Ubuntu wants me to disable
>    Secure Boot, and I don't want to do that.
> 
>  * Trying to use the libvirt plugin for vagrant requires I pick
>    from a very small set of possible images, and requires that I
>    install the upstream vagrant rather than the vagrant shipped
>    with Ubuntu 16.04
> 
>  * I find docker containers faster to work with and more transparent.
> 
> So I've done the work to make docker work for Patchwork development.
> This doesn't break or in any way interfere with using Vagrant, it just
> provides an alternative.
> 
> It includes support for headless selenium tests using Chrome.

Would it be possible to use chromium instead?  It's probably included in
distributions and wouldn't require adding a third-party repo, and worse,
proprietary software.

> 
> Signed-off-by: Daniel Axtens 
> ---
>  .dockerignore |  3 ++
>  README.md | 38 +++-
>  docker-compose.yml| 23 ++
>  docker/Dockerfile | 50 +++
>  docker/bashrc |  5 
>  docker/db/.dockerignore   |  1 +
>  docker/db/.gitignore  |  1 +
>  docker/db/Dockerfile  | 10 +++
>  docker/entrypoint.sh  | 76
> +++
>  docker/google-chrome.list |  1 +
>  10 files changed, 207 insertions(+), 1 deletion(-)
>  create mode 100644 .dockerignore
>  create mode 100644 docker-compose.yml
>  create mode 100644 docker/Dockerfile
>  create mode 100644 docker/bashrc
>  create mode 100644 docker/db/.dockerignore
>  create mode 100644 docker/db/.gitignore
>  create mode 100644 docker/db/Dockerfile
>  create mode 100755 docker/entrypoint.sh
>  create mode 100644 docker/google-chrome.list
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH 03/10] parsemail: Add series parsing

[Russell Currey]

On Mon, 2016-06-13 at 11:41 +0100, Stephen Finucane wrote:
> It is now possible to parse and store series, so do just that.
> The parsing at the moment is based on both RFC822 headers and
> subject lines.
> 
> Signed-off-by: Stephen Finucane <stephen.finuc...@intel.com>
> ---

There might have been some edge cases this fails to parse, but for everything I
saw series were parsed correctly.

Tested-by: Russell Currey <rus...@russell.cc>
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH 02/10] models: Add 'Series' model and related models

[Russell Currey]

On Mon, 2016-06-13 at 11:41 +0100, Stephen Finucane wrote:
> Add a series model. This model is intentionally very minimal to allow
> as much dynaminism as possible. It is expected that patches will be
> migrated between series as new data is provided.
> 
> Signed-off-by: Stephen Finucane 
> ---



> diff --git a/patchwork/models.py b/patchwork/models.py
> index 9ad2bcb..67ea012 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -37,6 +37,9 @@ from django.utils.six.moves import filter
>  from patchwork.fields import HashField
>  from patchwork.parser import extract_tags, hash_patch
>  
> +SERIES_REVISION_DEFAULT_NAME = 'Unnamed Series Revision'
> +SERIES_DEFAULT_NAME = 'Unnamed Series'
> +
>  

Having multiple series revisions without a cover letter appear the same is a
problem.  If there are multiple series next to each other in the patch view,
it's visually difficult to tell them apart, even though they link to different
places. Perhaps series without a cover letter could instead be named after the
first patch in the series, prefixed with "Series starting with" or "[Missing
cover letter]", or something?
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH] parsemail: Ignore multiple cover letters

[Russell Currey]

If multiple cover letters are found, parsemail dies with a
MultipleObjectsReturned exception.  This is particularly problematic in
the middle of a parsearchive run, so just ignore multiple cover letters.

Signed-off-by: Russell Currey <rus...@russell.cc>
---
 patchwork/bin/parsemail.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
index 8648d29..48f809f 100755
--- a/patchwork/bin/parsemail.py
+++ b/patchwork/bin/parsemail.py
@@ -523,6 +523,9 @@ def parse_mail(mail, list_id=None):
 CoverLetter.objects.all().get(name=name)
 except CoverLetter.DoesNotExist:  # no match => new cover
 is_cover_letter = True
+except CoverLetter.MultipleObjectsReturned:
+# if multiple cover letters are found, just ignore
+pass
 else:
 is_cover_letter = True
 
-- 
2.9.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH] uwsgi: Add python plugin

[Russell Currey]

Given the documentation suggests using Ubuntu, and any Debian derivative
requires manually specifying plugins in uwsgi, add the python27 plugin to
the example uwsgi settings file.

Signed-off-by: Russell Currey <rus...@russell.cc>
---
 lib/uwsgi/patchwork.ini | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/uwsgi/patchwork.ini b/lib/uwsgi/patchwork.ini
index 4f5dc90..fc0f0ed 100644
--- a/lib/uwsgi/patchwork.ini
+++ b/lib/uwsgi/patchwork.ini
@@ -1,5 +1,7 @@
 [uwsgi]
 
+plugins = python27
+
 project = patchwork
 base = /opt
 user = www-data
-- 
2.9.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 2/2] docs/deployment: Fix systemctl commands

[Russell Currey]

systemctl takes the name of the operation before the name of the service
so that you can do multiple things at once, i.e. "systemctl status nginx
postgresql".  Fix the ordering of the arguments.

Signed-off-by: Russell Currey <rus...@russell.cc>
---
 docs/deployment.md | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/deployment.md b/docs/deployment.md
index e7b6004..9eae750 100644
--- a/docs/deployment.md
+++ b/docs/deployment.md
@@ -313,13 +313,13 @@ to start uWSGI at boot:
 
 Start the uWSGI service we created above:
 
-$ sudo systemctl uwsgi start
-$ sudo systemctl uwsgi status
+$ sudo systemctl start uwsgi
+$ sudo systemctl status uwsgi
 
 Next up, restart the nginx service:
 
-$ sudo systemctl nginx restart
-$ sudo systemctl nginx status
+$ sudo systemctl restart nginx
+$ sudo systemctl status nginx
 
 Patchwork uses a cron script to clean up expired registrations and send
 notifications of patch changes (for projects with this enabled). Something like
-- 
2.9.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH 1/2] docs/deployment: Fix uwsgi service heredoc example

[Russell Currey]

The uwsgi service command didn't work without root, fix it by running the
entire expression under a shell with root privileges.

Signed-off-by: Russell Currey <rus...@russell.cc>
---
 docs/deployment.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/deployment.md b/docs/deployment.md
index 73282f9..e7b6004 100644
--- a/docs/deployment.md
+++ b/docs/deployment.md
@@ -290,7 +290,7 @@ boots, in addition to any time it may fail. We can automate 
this process using
 systemd. To this end a [systemd unit file][ref-uwsgi-systemd] should be created
 to start uWSGI at boot:
 
-$ sudo cat << EOF > /etc/systemd/system/uwsgi.service
+$ sudo sh -c "cat >/etc/systemd/system/uwsgi.service" <<'EOF'
 [Unit]
 Description=uWSGI Emperor service
 
-- 
2.9.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH 00/10] Add series support

[Russell Currey]

On Thu, 2016-06-16 at 09:21 +1000, Andrew Donnellan wrote:
> On 15/06/16 19:07, Finucane, Stephen wrote:



> > The best option we might have, if per-series reporting is really
> > necessary, is to allow Check uploading against a Series endpoint. This
> > would actually cause N Checks to be created - one for each Patch in the
> > series - meaning each Patch could still be individually queried. It
> > would be a bit of a lie (we didn't actually test the patch by itself,
> > therefore it might be broken) and I wouldn't promote this workflow
> > myself (bisectability FTW), but it could be a good way of dealing with
> > extremely long-running or resource-intensive test suites, where
> > per-patch validation would be too expensive.
> 
> I suppose this would be better than nothing, though apart from being 
> mildly misleading it would also generate a lot of spam. If I'm 
> submitting 5 test results against the entire series and 1-2 test results 
> against each individual patch, and we have all of those results 
> appearing on every patch, things could get more than a little bit 
> confusing. Personally, I'd be inclined to submit series-wide test 
> results as checks on the very last patch in the series instead of using 
> this.
> 
I don't think adding checks on the very last patch is particularly intuitive.
Having a Series endpoint that creates checks on every patch in the series would
be fine so long as there was some obvious way of distinguishing what was run on
the series as a whole and what was run on the individual patch, whether it's
something arbitrary like "[series]" in the check name or if there was some flag
that triggered a visual differentiation.

The reason I brought this up is that Andrew and I wrote a tool to automatically
test series as they're indexed in Patchwork that was built on the Series support
in the fdo fork, I'll play around with this series and see if anything major is
missing that would prevent us from doing the same with this implementation.
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork