aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* 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>
* docs: Reference Patchwork 2.2 tarballsStephen Finucane2020-04-08
| | | | | | This is what users will want right now. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Remove 'earliest_version' configStephen Finucane2020-04-05
| | | | | | | This is unnecessary and was disabling reno's built-in ability to detect a base branch. Signed-off-by: Stephen Finucane <stephen@that.guru>
* tox: Pass positional arguments to 'docs' jobStephen Finucane2020-04-05
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Update reno for stable/2.2Stephen Finucane2020-04-05
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* Post-release version bumpStephen Finucane2020-04-05
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* Release 2.2.0Stephen Finucane2020-04-05
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* trivial: Feed 'patchwork.migrations' through blackStephen Finucane2020-03-19
| | | | | | | Include migrations in the flake8 checks, which should catch some simple undefined variables errors. Signed-off-by: Stephen Finucane <stephen@that.guru>
* REST: Add release note for faster queriesDaniel Axtens2020-03-19
| | | | | | | | Didn't quite seem like it fit anywhere else in the series. I want the release note mostly because I hope to backport this to stable. Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
* REST: extend performance improvements to other parts of the APIDaniel Axtens2020-03-19
| | | | | | | | | | We can trivially extend what we've just done to other parts of the API. I haven't done much by way of benchmark but we're seeing multiple 'x's pretty much across the board when filtering. Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
* REST: fix patch listing queryDaniel Axtens2020-03-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The patch listing query is punishingly slow under even very simple filters. The new data model in 3.0 will help _a lot_, so this is a simple fix: I did try indexes but haven't got really deeply into the weeds of what we can do with them. Move a number of things from select_related to prefetch_related: we trade off one big, inefficient query for a slightly larger number of significantly more efficient queries. On my laptop with 2 copies of the canonical kernel team list loaded into the database, and considering only the API view (the JSON view is always faster) with warm caches and considering the entire set of SQL queries: - /api/patches/?project=1 ~1.4-1.5s -> <100ms, something like 14x better - /api/patches/?project=1&since=2010-11-01T00:00:00 ~1.7-1.8s -> <560ms, something like 3x better (now dominated by the counting query only invoked on the HTML API view, not the pure JSON API view.) The things I moved: * project: this was generating SQL that looked like: INNER JOIN `patchwork_project` T5 ON (`patchwork_submission`.`project_id` = T5.`id`) This is correct but we've already had to join the patchwork_submission table and perhaps as a result it seems to be inefficient. * series__project: Likewise we've already had to join the series table, doing another join is possibly why it is inefficient. * delegate: I do not know why this was tanking performance. I think it might relate to the strategy mysql was using. Reported-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
* REST: massively improve the patch counting query under filtersDaniel Axtens2020-03-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The DRF web view counts the patches as part of pagination. The query it uses is a disaster zone: SELECT Count(*) FROM ( SELECT DISTINCT `patchwork_submission`.`id` AS Col1, `patchwork_submission`.`msgid` AS Col2, `patchwork_submission`.`date` AS Col3, `patchwork_submission`.`submitter_id` AS Col4, `patchwork_submission`.`project_id` AS Col5, `patchwork_submission`.`name` AS Col6, `patchwork_patch`.`submission_ptr_id` AS Col7, `patchwork_patch`.`commit_ref` AS Col8, `patchwork_patch`.`pull_url` AS Col9, `patchwork_patch`.`delegate_id` AS Col10, `patchwork_patch`.`state_id` AS Col11, `patchwork_patch`.`archived` AS Col12, `patchwork_patch`.`hash` AS Col13, `patchwork_patch`.`patch_project_id` AS Col14, `patchwork_patch`.`series_id` AS Col15, `patchwork_patch`.`number` AS Col16, `patchwork_patch`.`related_id` AS Col17 FROM `patchwork_patch` INNER JOIN `patchwork_submission` ON (`patchwork_patch`.`submission_ptr_id`=`patchwork_submission`.`id`) WHERE `patchwork_submission`.`project_id`=1 ) This is because django-filters adds a DISTINCT qualifier on a ModelMultiChoiceFilter by default. I guess it makes sense and they do a decent job of justifying it, but it causes the count to be made with this awful subquery. (The justification is that they don't know if you're filtering on a to-many relationship, in which case there could be duplicate values that need to be removed.) While fixing that, we can also tell the filter to filter on patch_project rather than submission's project, which allows us in some cases to avoid the join entirely. The resultant SQL is beautiful when filtering by project only: SELECT COUNT(*) AS `__count` FROM `patchwork_patch` WHERE `patchwork_patch`.`patch_project_id` = 1 On my test setup (2x canonical kernel mailing list in the db, warm cache, my laptop) this query goes from >1s to ~10ms, a ~100x improvement. If we filter by project and date the query is still nice, but still also very slow: SELECT COUNT(*) AS `__count` FROM `patchwork_patch` INNER JOIN `patchwork_submission` ON (`patchwork_patch`.`submission_ptr_id`=`patchwork_submission`.`id`) WHERE (`patchwork_patch`.`patch_project_id`=1 AND `patchwork_submission`.`date`>='2010-11-01 00:00:00') This us from ~1.3s to a bit under 400ms - still not ideal, but I'll take the 3x improvement! Reported-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
* Patchwork v2.2.0-rc2Stephen Finucane2020-03-16
| | | | | Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* REST: Add patch relationsMete Polat2020-03-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | View relations and add/update/delete them as a maintainer. Maintainers can only create relations of patches which are part of a project they maintain. Because this is a writable many-many nested relationship, it behaves a little unusually. In short: - All operations use PATCH to the 'related' field of a patch - To relate a patch to another patch, say 7 to 19, either: PATCH /api/patch/7 related := [19] PATCH /api/patch/19 related := [7] - To delete a patch from a relation, say 1, 21 and 42 are related but we only want it to be 1 and 42: PATCH /api/patch/21 related := [] * You _cannot_ remove a patch from a relation by patching another patch in the relation: I'm trying to avoid read-modify-write loops. * Relations that would be left with just 1 patch are deleted. This is only ensured in the API - the admin interface will let you do this. - Break-before-make: if you have [1, 12, 24] and [7, 15, 42] and you want to end up with [1, 12, 15, 42], you have to remove 15 from the second relation first: PATCH /api/patch/1 related := [15] will fail with 409 Conflict. Instead do: PATCH /api/patch/15 related := [] PATCH /api/patch/1 related := [15] -> 200 OK, [1, 12, 15, 42] and [7, 42] are the resulting relations Signed-off-by: Mete Polat <metepolat2000@gmail.com> Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* models, templates: Add patch relationsMete Polat2020-03-16
| | | | | | | | | | | | | | | | Introduces the ability to add relations between patches. Relations are displayed in the details page of a patch under 'Related'. Related patches located in another projects can be viewed as well. Changes to relations are tracked in events. Currently the display of this is very bare in the API but that will be fixed in a subsequent patch: this is the minimum required to avoid throwing errors when you view the events feed. Signed-off-by: Mete Polat <metepolat2000@gmail.com> [dja: address some review comments from Stephen, add an admin view, move to using Events, misc tidy-ups.] Signed-off-by: Daniel Axtens <dja@axtens.net>
* tests: Add tests for invalid cover/patch IDsStephen Finucane2020-03-12
| | | | | Signed-off-by: Stephen Finucane <stephen@that.guru> Related: #343
* views: Return Http404 if patch not foundAndriy Gelman2020-03-11
| | | | | | | | Otherwise exception DoesNotExist shows error 500 on Apache Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com> Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: #343
* migrations: Don't attempt to rehome patchesStephen Finucane2020-03-11
| | | | | | | | | | | | | | | | | Migration 0039 attempts to move patches that have ended up in an arbitrary series due to race conditions into the correct series. However, there are a number of race conditions that can occur here that make this particularly tricky to do. Given that series are really just arbitary metadata, it's not really necessary to do this...so don't. Instead, just delete the series references that identical message IDs and below to the same project, allowing us to add the uniqueness constraint and prevent the issue bubbling up in the future. This means we're still left with orphaned series but these could be fixed manually, if necessary. Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-by: Ali Alnubani <alialnu@mellanox.com> Closes: #340
* parser: Don't group patches with different versions in a seriesStephen Finucane2020-03-11
| | | | | | | | | | | | | | | | | | | As noted in #340 [1], if a patch from a series is dropped or miscategorised, patches from a later revision of that series can end up included in the earlier series rather than in their own series. This was actually intentional as part of the fix for #105 [2]. However, completely ignoring this information can be problematic. Refine things by checking for versions and, if they don't match, using timeboxing to try guess if they should be kept together. This would resolve the issue seen in #340 while preventing a regression for #105. [1] https://github.com/getpatchwork/patchwork/issues/340 [1] https://github.com/getpatchwork/patchwork/issues/105 Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-by: Ali Alnubani <alialnu@mellanox.com> Related: #340 Related: #105
* models: Handle unset 'Submission.content'Stephen Finucane2020-03-02
| | | | | | | | It's unlikely but 'Submission.content' is nullable and we weren't handling this here. Fix that. Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 7f6685a2a ("Fix CRLF newlines upon submission changes")
* docs: Reference current list-idStephen Finucane2020-02-28
| | | | | | | | We've updated our fixtures to use this newer version, meaning we no longer need to pass an explicit '--list-id' argument. Lovely. Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 9a0b5992 ("fixtures: Update Patchwork list ID")
* Add support for djangorestframework 3.11Stephen Finucane2020-02-28
| | | | | | There are no breaking changes apparent. Signed-off-by: Stephen Finucane <stephen@that.guru>
* Update jinja2 from 2.10.1 to 2.11.1pyup-bot2020-02-27
|
* tox: Switch non-pyNN targets to Python 3Stephen Finucane2020-02-27
| | | | | | | This is long overdue and highlights a small issue, which is easily fixed by use of Sphinx. Signed-off-by: Stephen Finucane <stephen@that.guru>