| Commit message (Collapse) | Author | Age |
... | |
|
|
|
|
|
|
|
|
|
| |
I occasionally forget myself and run e.g. 'tox -e pyNN' when I want to
sanity check something instead of 'tox -e pyNN-djangoMM'. Add fallback
Django versions so that this doesn't crash and burn. It's less succict
than it could be since tox doesn't seem to support '!django{22,30}'
(yet!).
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
| |
This should have been in the 'patchwork/migrations' directory, not
'migrations.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Fixes: 9bb5caec ("migrations: Squash 0001 to 0040")
|
|
|
|
|
|
|
|
|
|
| |
0042 was using MySQL-specific SQL to delete entries in the
'patchwork_comment' table that were associated with entries in the
'patchwork_coverletter' table, while 0043 only considered MySQL and
PostgrSQL when attempting to copy fields from 'patchwork_patch' to
'patchwork_submission'. Both issues are resolved.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Now that we're moving a new major version, we can squash the migrations
we have to date. This coexists alongside the old migrations, which we
can remove and replace entirely when we release 4.0, per the advice of
the 'squashmigrations' tool.
You should commit this migration but leave the old ones in place; the
new migration will be used for new installs. Once you are sure all
instances of the codebase have applied the migrations you squashed,
you can delete them.
The 'squashmigrations' tool can't parse the output of 'RunPython' blocks
so the output of the tool was less optimized than it could be. As a
result, we've manually modified this change to remove the 'RunPython'
block and unnecessary 'AlterField' entries. This was done by removing
all migrations and generating a new "initial" migration, which was then
modified to mark all strings as byte strings (as they were when we were
using Python 2 to generate these migrations) so that 0041 would apply
cleanly.
The main benefit of this change is that it significantly reduces the
startup time for unit tests. Executed on my host, the run time for a
single test goes from ~ 22 seconds to ~ 14 seconds. This is obviously
reduced for additional tests.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Oh, the follies of youth. Time to undo the damage of 2.0.0, specifically
commit 86172ccc16, which split Patch into two separate models using
concrete inheritance. As noted previously, this introduced a large
number of unavoidable JOINs across large tables and the performance
impacts these introduce are blocking other features we want, such as
improved tagging functionality. To combine these two models, we must do
the following:
- Update any references to the 'Patch' model to point to the
'Submission' model instead
- Move everything from 'Patch' to 'Submission', including both fields
and options
- Delete the 'Patch' model
- Rename the 'Submission' model to 'Patch'
With this change, our model "hierarchy" goes from:
Submission
Patch
PatchComment
Cover
CoverComment
To a nice, flat:
Patch
PatchComment
Cover
CoverComment
I expect this migration to be intensive, particularly for MySQL users
who will see their entire tables rewritten. Unfortunately I don't see
any way to resolve this in an easier manner.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We want to get rid of the split between 'Patch' and 'Submission' because
of the cost of using JOINs basically everywhere we use 'Patch'. Before
we do that, we need to move the other users of 'Submission' to other
models and other models that rely on these users sharing the common
'Submission' base. For the former, there is only one user,
'CoverLetter', while for the latter there is only the 'Comment' model.
As a result, we must do the following:
- Create a new 'Cover' model
- Create a new 'CoverComment' model
- Move everything from 'CoverLetter' to 'Cover' and all entries
associated with a 'CoverLetter' from 'Comment' to 'CoverComment'
- Delete the 'CoverLetter' model
- Rename the 'Comment' model to 'PatchComment'
This means our model "hierarchy" goes from:
Submission
Patch
CoverLetter
Comment
To:
Submission
Patch
PatchComment
Cover
CoverComment
A future change will flatten the 'Submission' and 'Patch' model.
Note that this actually highlighted a bug in Django, which has since
been reported upstream [1]. As noted there, the issue stems from MySQL's
refusal to remove an index from a foreign key when DB constraints are
used and the workaround is to remove the foreign key constraint before
altering the indexes and then re-add the constraint after.
[1] https://code.djangoproject.com/ticket/31335
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This reverts commit cfcf2f2a80ac0709f1a5fd9aa212c8403daa5a18.
This will no longer be necessary once we remove the Patch-Submission
split. Revert it now to avoid needing to rejig this later.
Conflicts:
patchwork/views/project.py
NOTE(stephenfin): Conflicts are due to commit 880ec8c5 ("Fetch
maintainer information in one query") which changed nearby lines.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
| |
We want to drop this in future changes. Start by removing any
unnecessary references.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
| |
We're going to be doing some model surgery shortly. Do the necessary
renaming of variables ahead of this.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
| |
This should have been removed in commit f1e089f773.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The following were reported by Travis' build config validation:
- root: deprecated key 'sudo' (The key `sudo` has no effect anymore.)
- env: key 'matrix' is an alias for 'jobs', using 'jobs'
- root: key 'matrix' is an alias for 'jobs', using 'jobs'
- root: missing 'os', using the default 'linux'
Resolve all of the above.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
| |
As we've done for the Patch and Comment models, this change prevents
database errors from duplicate CoverLetters.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Stephen Finucane <stephen@that.guru>
[stephenfin: Add release note]
|
|
|
|
|
|
|
|
| |
As we've done for the Patch model, this change prevents database errors
from duplicate Comments.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Reviewed-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, the parser relies on causing (and catching) IntegrityErrors
on patch insert to catch duplicate (msgid,project) mails.
This change performs an atomic select -> insert instead.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Stephen Finucane <stephen@that.guru>
[stephenfin: Remove 'expectedFailure' marker again]
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, the parser causes IntegrityErrors while inserting duplicate
patches; these tend to pollute database logs.
This change adds a check, which currently fails, to ensure we do not
cause errors during a duplicate patch parse.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Stephen Finucane <stephen@that.guru>
[stephenfin: Add 'expectedFailure' marker to keep all tests green]
|
|
|
|
|
|
|
|
| |
Test that we get the correct DuplicateMailError from parsing the same
mail twice.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Reviewed-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
get_original_sender() tries to demangle DMARC-mangled From headers, in
the case where the email's From address is the list address. It knows how
to handle Google Groups and Mailman style mangling, where the original
submitter's name will be turned into e.g. "Andrew Donnellan via
linuxppc-dev".
If an email has the From header set to the list address but has a name that
doesn't include " via ", we'll throw an exception because stripped_name
hasn't been set. Sometimes this is because the list name is seemingly
empty, resulting in a mangled name like "Andrew Donnellan via"
without the space after "via" that we detect. Handle this as well as we can
instead, and add a test.
Fixes: 8279a84238c10 ("parser: Unmangle From: headers that have been mangled for DMARC purposes")
Reported-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
Reviewed-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
| |
No changes necessary, thankfully, though there is a feature gap here
that we will need 0.7.0 to close [1] :(
[1] https://github.com/sphinx-contrib/openapi/pull/87
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We've done the necessary work here already so this is a relatively easy
switchover. However, we do have to work around an issue whereby the
first possible matching route is used rather than the best one [1]. In
addition, we have to install from master since there are fixes missing
from the latest release, 0.13.3. Hopefully both issues will be resolved
in a future release.
[1] https://github.com/p1c2u/openapi-core/issues/226
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
| |
openapi-core 0.13.x has added support for Django validation. Before we
migrate to that version and presumably remove most of this code, remove
the stuff that is *definitely* dead.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
| |
Two issues here:
- 'PATCH /patches/{id}' and 'PUT /patches/{id}' expect a list of
integers on the 'related' field - not strings
- 'GET /patches' and 'GET /patches/{id}' return a list of embedded patch
objects on the 'related' field - not strings
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Four things to change here:
- The response is any array that can contain any type of event, not one
of them.
- The 'actor' field is nullable.
- The 'cover' field of the 'cover-created' event is an embedded cover
letter, not a string.
- The specifications for the 'current_delegate' and 'previous_delegate'
fields of the 'patch-delegated' field were apparently invalid.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
| |
Each header in the 'headers' field can be either a string or a list
value.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
| |
Two issues:
- Errors are reported as a mapping of the field name to an array of
errors, not a string.
- We were attempting to validate an invalid request.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
| |
Two issues to correct:
- Each header in the 'headers' field can be either a string or a list
value.
- We state that the 'content' field will always have content but our
tests were configuring otherwise.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
| |
Errors are reported as a mapping of the field name to an array of
errors, not a string.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
| |
Two issues here:
- The ID in '/projects/{id}' can be either an integer or a string.
- We were attempting to validate an invalid request.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Four issues to resolve:
- The 'tags' field is a key-value mapping, not an array.
- Each header in the 'headers' field can be either a string or a list
value.
- Errors are reported as a mapping of the field name to an array of
errors, not a string.
- The security type information isn't complete and doesn't account for
security types. Skip it for now.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
| |
Turns out you don't have to nest common elements under individual routes
[1]. Less duplication and more sensible docs = winning.
[1] https://swagger.io/specification/#pathItemObject
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
| |
As discussed at [1], "subtypes can add restrictions, but they cannot
relax restrictions that are already in place." These fields need to be
marked nullable and then "subclassed" to set non-nullability if
required.
[1] https://github.com/OAI/OpenAPI-Specification/issues/1368
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
| |
Just to make sure we're not generating garbage.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
| |
Now that pyup properly supports compatible ranges [1], it's time to
remove these markers.
[1] https://github.com/pyupio/pyup/pull/367
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
| |
Presently, when updating a patch we assume that patches are provided.
This isn't necessary - you might just want to make it public - and isn't
enforced by the API itself. However, because we make this assumption, we
see a HTTP 500. Resolve the issue and add tests to prevent a regression.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Resolves: #357
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In the process of fixing the previous bug, I realised that:
a) /api/patches/msgid is a perfectly reasonable thing to attempt
b) We have no way of finding a patch by message id in the API
We can't actualy make /api/patches/msgid work because it may not
be unique, but we can add a filter.
I'm shoehorning this into stable/2.2, even though it's technically
an API change: it's minor, not incompatible and in hindsight a
glaring hole.
Cc: Michael Ellerman <mpe@ellerman.id.au>
Tested-by: Jeremy Kerr <jk@ozlabs.org>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
Reviewed-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Daniel Axtens <dja@axtens.net>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
mpe and jk and sfr found that the OzLabs server was melting due
to some queries downloading every patch.
Turns out if you 404 the patch detail view in the API, d-r-f attempts
to render a listbox with every single patch to fill in the 'related'
field. The bundle API also has a similar field.
Replace the multiple selection box with a text field. You can still
(AIUI) populate the relevant patch IDs manually.
This is the recommended approach per
https://www.django-rest-framework.org/topics/browsable-api/#handling-choicefield-with-large-numbers-of-items
Reported-by: Jeremy Kerr <jk@ozlabs.org>
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Tested-by: Jeremy Kerr <jk@ozlabs.org>
Server-no-longer-on-fire-by: Jeremy Kerr <jk@ozlabs.org>
Reviewed-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Daniel Axtens <dja@axtens.net>
|
|
|
|
|
|
|
|
| |
This lets us use e.g. f-strings. We also bump the version of the default
pre-commit lib and migrate to the upstream flake8 plugin, since the old
one is now deprecated.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change fixes a few omissions in the grant scripts:
- patchrelation is missing from both mysql and postgres scripts; it's
only needed for web user access.
- event is missing from the web grants on postgres, and the mail grants
on mysql.
Tested on postgres only.
Fixes: 27c2acf56c ("models, templates: Add patch relations")
Fixes: 34e3c9c493 ("sql: Update 'grant-all.mysql' script with missing tables")
Fixes: 234bc7c316 ("lib/sql: fix permissions for v2.0.0 on postgres")
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Daniel Axtens <dja@axtens.net>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
| |
This was preventing us from doing e.g.:
tox -e py36-django30 -- patchwork -v 2
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This resolves the following irritating warnings that were popping up on
Python 3.7 and 3.8 and were silenced on 3.6:
/usr/lib/python3.7/unittest/suite.py:107: ResourceWarning: unclosed <socket.socket ...>
Note that we need to use a subclass because the 'ServerProxy' class,
rather annoyingly, does not expose a 'close()' method. Instead, you're
expected to use a context manager, which isn't useful from the context
of a 'setUp' call. We could call '__enter__' and '__exit__' manually but
this seems cleaner. Also note that 'Server' was an alias of
'ServerProxy' [1], and we're taking the opportunity to switch here.
[1] https://docs.python.org/3/library/xmlrpc.client.html#xmlrpc.client.ServerProxy
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
| |
All of these are defaults in Python 3 [1].
[1] https://docs.python.org/3.6/library/__future__.html
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
| |
This does not support Django 3.0 yet [1].
[1] https://github.com/django-dbbackup/django-dbbackup/issues/314
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
| |
Django has wanted this for a long time. It doesn't seem to do anything
but it keeps Django happy so...
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
| |
This version formally includes support for Django 3.0 [1].
The default installed version of Django is bumped to 3.0.
[1] https://django-debug-toolbar.readthedocs.io/en/latest/changes.html
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
| |
Add the latest version of Django.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Stephen Finucane <stephen@that.guru>
Closes: #311
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Django 3.0 switches to using Python 3's built-in HTML escaper, which
prefers to escape entities using hex rather than decimal.
Some of our tests check rendered HTML output against pre-escaped
strings, and fail because ''' is now '''.
Fix this by using the escape function so we get consistent escaping no
matter which Django version.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
Reviewed-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
| |
We no longer need these dependencies.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|