[v2,0/9] Add support for Django 1.11

This is a sizable, albeit mostly trivial series focused on (a) adding support
for Django 1.11 to Patchwork. Previously this also contained a series of
patches aimed at resolving all 'DeprecationWarning's for the versions of Django
that we support, but these have since been merged separately.

Changes since v1:
- Drop most "fix deprecation warning" patches, since these are already merged

Stephen Finucane (9):
  models: Remove 'permalink' decorator
  urls: Use new login/password change CBVs
  trivial: noqa imports dotted through urls
  tox: Remove '--liveserver' parameter
  migrations: Mark 'RunPython' blocks as non-atomic
  REST: Allow for mutability of request.POST
  tox: Add Django 1.11
  README: Add support for Django 1.11
  requirements: Enable Django 1.11

 README.rst                                         |   2 +-
 patchwork/api/check.py                             |   9 ++
 .../0007_move_comment_content_to_patch_content.py  |   6 +-
 patchwork/migrations/0016_series_project.py        |   2 +-
 patchwork/models.py                                |  17 ++-
 patchwork/urls.py                                  | 116 ++++++++++++++-------
 requirements-dev.txt                               |   2 +-
 requirements-prod.txt                              |   2 +-
 tox.ini                                            |  10 +-
 9 files changed, 112 insertions(+), 54 deletions(-)

Hi Stephen,

> This is a sizable, albeit mostly trivial series focused on (a) adding support
> for Django 1.11 to Patchwork. Previously this also contained a series of
> patches aimed at resolving all 'DeprecationWarning's for the versions of Django
> that we support, but these have since been merged separately.

The series, with the modifications in my other emails, is:
Tested-by: Daniel Axtens <dja@axtens.net>

I haven't specifically examined the patches to the level I'm comfortable
with giving them Reviews, but I am confident that they work.

Earlier you talked about a performance regression with 1.11 - did you
ever find out anything about that? I have been looking at doing a proper
performance test suite, but haven't been able to carve out a chunk of
time to do it...

Regards,
Daniel

On Thu, 2017-11-16 at 21:02 +1100, Daniel Axtens wrote:
> Hi Stephen,
> 
> > This is a sizable, albeit mostly trivial series focused on (a)
> > adding support
> > for Django 1.11 to Patchwork. Previously this also contained a
> > series of
> > patches aimed at resolving all 'DeprecationWarning's for the
> > versions of Django
> > that we support, but these have since been merged separately.
> 
> The series, with the modifications in my other emails, is:
> Tested-by: Daniel Axtens <dja@axtens.net>

Thanks for the reviews, Daniel. This is all merged now.

> I haven't specifically examined the patches to the level I'm
> comfortable with giving them Reviews, but I am confident that they
> work.
> 
> Earlier you talked about a performance regression with 1.11 - did you
> ever find out anything about that? I have been looking at doing a
> proper performance test suite, but haven't been able to carve out a
> chunk of time to do it...

Aye, this was the issue I pointed out with the '/api/events' endpoint.
As you've noted, this is unrelated to Django 1.11 and has to be
resolved separately (I'm working on it).

Stephen

Stephen Finucane <stephen@that.guru> writes:

> On Thu, 2017-11-16 at 21:02 +1100, Daniel Axtens wrote:
>> Hi Stephen,
>> 
>> > This is a sizable, albeit mostly trivial series focused on (a)
>> > adding support
>> > for Django 1.11 to Patchwork. Previously this also contained a
>> > series of
>> > patches aimed at resolving all 'DeprecationWarning's for the
>> > versions of Django
>> > that we support, but these have since been merged separately.
>> 
>> The series, with the modifications in my other emails, is:
>> Tested-by: Daniel Axtens <dja@axtens.net>
>
> Thanks for the reviews, Daniel. This is all merged now.

Fantastic, thanks.
>
>> I haven't specifically examined the patches to the level I'm
>> comfortable with giving them Reviews, but I am confident that they
>> work.
>> 
>> Earlier you talked about a performance regression with 1.11 - did you
>> ever find out anything about that? I have been looking at doing a
>> proper performance test suite, but haven't been able to carve out a
>> chunk of time to do it...
>
> Aye, this was the issue I pointed out with the '/api/events' endpoint.
> As you've noted, this is unrelated to Django 1.11 and has to be
> resolved separately (I'm working on it).

I have made some (so far incomplete) attempts centering around
denormalising the project field - moving it either completely out of
submission and into patch/coverletter/comment or just duplicating it in
those three models. This also fixes the fact that we need a SQL join
just to enumerate patches which is slow (and an OzLabs pain point) - but
the migrations are a paaaaaiiiiiin. Is that your approach or do you have
other ideas?

Regards,
Daniel

>
> Stephen

On Mon, 2017-12-04 at 09:29 +1100, Daniel Axtens wrote:
> Stephen Finucane <stephen@that.guru> writes:
> 
> > On Thu, 2017-11-16 at 21:02 +1100, Daniel Axtens wrote:

[snip]

> > > I haven't specifically examined the patches to the level I'm
> > > comfortable with giving them Reviews, but I am confident that
> > > they work.
> > > 
> > > Earlier you talked about a performance regression with 1.11 - did
> > > you ever find out anything about that? I have been looking at
> > > doing a proper performance test suite, but haven't been able to
> > > carve out a chunk of time to do it...
> > 
> > Aye, this was the issue I pointed out with the '/api/events'
> > endpoint. As you've noted, this is unrelated to Django 1.11 and has
> > to be resolved separately (I'm working on it).
> 
> I have made some (so far incomplete) attempts centering around
> denormalising the project field - moving it either completely out of
> submission and into patch/coverletter/comment or just duplicating it
> in those three models. This also fixes the fact that we need a SQL
> join just to enumerate patches which is slow (and an OzLabs pain
> point) - but the migrations are a paaaaaiiiiiin. Is that your
> approach or do you have other ideas?

Not quite. I've been focused on the 'Event' model mostly and getting
rid of the most of the ForeignKeys contained therein. To do this, I'm
replacing the cover letter and patch foreign keys with a submission
reference and storing a serialized JSON blob for things like
'previous_delegate' (the payload) instead. This tightly binds us to the
REST API representation and means we lose things the integrity
constraints that foreign keys provide. However, we only expose this
stuff over that API and we don't generally delete anything so the
constraints don't give us much. As for migrations, I've got a feature
to automatically delete ("expire") events after an interval (defaulting
to 30 days). The main point of this is that events lose value the older
they are, but I figure if this run before the migration, it would limit
the amount of things we have to migrate.

I haven't yet looked at removing the join for patches and cover
letters, but have you thought of simply folding these in and using an
enum-like field to indicate if it's a cover letter or patch? The
migration for that is also going to be tough and it's going to
denormalize things, but if the JOINs are a serious issue it might be
the only option we have.

Stephen