aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* models: Merge 'Patch' and 'Submission'Stephen Finucane2020-04-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* models: Split 'CoverLetter' from 'Submission'Stephen Finucane2020-04-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Revert "Be sensible computing project patch counts"Stephen Finucane2020-04-26
| | | | | | | | | | | | | | | 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>
* Remove unnecessary references to Submission modelStephen Finucane2020-04-26
| | | | | | | We want to drop this in future changes. Start by removing any unnecessary references. Signed-off-by: Stephen Finucane <stephen@that.guru>
* trivial: Rename 'CoverLetter' references to 'Cover'Stephen Finucane2020-04-26
| | | | | | | 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>
* forms: Remove LoginFormStephen Finucane2020-04-23
| | | | | | This should have been removed in commit f1e089f773. Signed-off-by: Stephen Finucane <stephen@that.guru>
* travis: Resolve warnings, info messages from TravisStephen Finucane2020-04-18
| | | | | | | | | | | | | 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>
* parser: don't trigger database IntegrityErrors on duplicate coverlettersJeremy Kerr2020-04-18
| | | | | | | | | 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]
* parser: don't trigger database IntegrityErrors on duplicate commentsJeremy Kerr2020-04-18
| | | | | | | | 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>
* parser: prevent IntegrityErrorsJeremy Kerr2020-04-18
| | | | | | | | | | | 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]
* tests: ensure we don't see database errors during duplicate insertJeremy Kerr2020-04-18
| | | | | | | | | | | | 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]
* tests: Add duplicate mail testJeremy Kerr2020-04-18
| | | | | | | | 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>
* parser: Don't crash when From: is list email but has weird mangle formatAndrew Donnellan2020-04-18
| | | | | | | | | | | | | | | | | | | | 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>
* docs: Update sphinxcontrib-openapiStephen Finucane2020-04-18
| | | | | | | | | 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>
* tests: Switch to openapi-core 0.13.xStephen Finucane2020-04-18
| | | | | | | | | | | | | 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>
* tests: Drop Django 1.x supportStephen Finucane2020-04-18
| | | | | | | | 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>
* docs: Resolve issues with 'relations'Stephen Finucane2020-04-18
| | | | | | | | | | | 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>
* docs: Resolve issues with 'events'Stephen Finucane2020-04-18
| | | | | | | | | | | | | | 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>
* docs: Resolve issues with 'comments'Stephen Finucane2020-04-18
| | | | | | | Each header in the 'headers' field can be either a string or a list value. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Resolve issues with 'patches'Stephen Finucane2020-04-18
| | | | | | | | | | 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>
* docs: Resolve issues with 'covers'Stephen Finucane2020-04-18
| | | | | | | | | | | 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>
* docs: Resolve issues with 'bundles'Stephen Finucane2020-04-18
| | | | | | | 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>
* docs: Resolve issues with 'projects'Stephen Finucane2020-04-18
| | | | | | | | | 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>
* docs: Resolve issues with 'patches'Stephen Finucane2020-04-18
| | | | | | | | | | | | | | 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>
* docs: Move common path parameters to parentStephen Finucane2020-04-18
| | | | | | | | | 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>
* docs: Make embedded fields nullable by defaultStephen Finucane2020-04-18
| | | | | | | | | | | 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>
* docs: Add schema validation to 'generate-schemas' toolStephen Finucane2020-04-18
| | | | | | Just to make sure we're not generating garbage. Signed-off-by: Stephen Finucane <stephen@that.guru>
* requirements: Remove final pyup markersStephen Finucane2020-04-18
| | | | | | | | | 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>
* REST: Allow update of bundle without patchesStephen Finucane2020-04-18
| | | | | | | | | | 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
* api: allow filtering patches and covers by msgidDaniel Axtens2020-04-14
| | | | | | | | | | | | | | | | | | | | 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>
* api: do not fetch every patch in a patch detail view 404Daniel Axtens2020-04-14
| | | | | | | | | | | | | | | | | | | | | | | 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>
* pre-commit: Use Python 3 for everythingStephen Finucane2020-04-14
| | | | | | | | 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>
* lib/sql: Update grant script for recent schema changesJeremy Kerr2020-04-14
| | | | | | | | | | | | | | | | | | 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>
* Additional Python 2.7 cleanupsStephen Finucane2020-04-09
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* tox: Remove quotes around posargsStephen Finucane2020-04-09
| | | | | | | | This was preventing us from doing e.g.: tox -e py36-django30 -- patchwork -v 2 Signed-off-by: Stephen Finucane <stephen@that.guru>
* tests: Close XML-RPC client when doneStephen Finucane2020-04-09
| | | | | | | | | | | | | | | | | | 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>
* Remove __future__ importsStephen Finucane2020-04-09
| | | | | | | | 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>
* Temporarily disable django-dbbackupStephen Finucane2020-04-09
| | | | | | | | 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>
* migrations: Add the Python 3 patchStephen Finucane2020-04-08
| | | | | | | 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>
* requirements: Bump django-debug-toolbar to 2.2Stephen Finucane2020-04-08
| | | | | | | | | | 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>
* REST: Resolve warnings with DRF >= 3.11Stephen Finucane2020-04-08
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* Add Django 3.0 supportAndrew Donnellan2020-04-08
| | | | | | | | 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
* tests: Fix escaping in bundle tests on Django 3.0Andrew Donnellan2020-04-08
| | | | | | | | | | | | | | 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 '&#39;' is now '&#x27;'. 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>
* docker: Remove Python 2.7, 3.5Stephen Finucane2020-04-08
| | | | | | We no longer need these dependencies. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Remove references to Python 2.7Stephen Finucane2020-04-08
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* Clean up references to Python 2.7, Python 3.5Daniel Axtens2020-04-08
| | | | | | | | Both this and the version of Django we were running with it are EOL upstream. It's time to drop them. Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru>
* Remove unnecessary compat wrappersStephen Finucane2020-04-08
| | | | | | | This will probably need to be reintroduced with future Django versions, but for now it's gone. Signed-off-by: Stephen Finucane <stephen@that.guru>
* Replace references to Django 1.11 docsStephen Finucane2020-04-08
| | | | | | | This is a straight forward swap, thankfully. Django 2.2 is chosen as it's the latest LTS. Signed-off-by: Stephen Finucane <stephen@that.guru>
* tox: Drop support for Django < 2.2, Python < 3.6Stephen Finucane2020-04-08
| | | | | | | | | Each of these versions of Django is now EOL, and Python 3.5 will be EOL by time we release the next version. Drop it. The Python 2.7 cleanup will be done separately. Signed-off-by: Stephen Finucane <stephen@that.guru>
* Revert "pyenv: also install requirements for python2"Stephen Finucane2020-04-08
| | | | | | | This reverts commit 5b904f91c4cab1ff3f8460c9d4ed920a990c8db3. It is no longer necessary now that we're dropping Python 2.7 support. Signed-off-by: Stephen Finucane <stephen@that.guru>