aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* 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>
* pyenv: also install requirements for python2Daniel Axtens2020-02-27
| | | | | | | | | | The first time you do a migration with python3, you get a whole lot of seemingly null changes. This is a bit annoying so I use py2 to generate the changes. To do that, first fix the pyenv transition so requirements are still installed for python2. Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
* docs: Update schemas to reflect latest changes in templateStephen Finucane2020-02-27
| | | | | | | | Looks like I forgot to run the 'generate-schema' script beforehand. Fix that now. Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: cd3a2ce8 ("REST: Allow configuration of user settings via API")
* docker: update dependency for current buildPranav Annam2020-02-17
| | | | | | | | | | | | | | | | | | | The dependency libssl1.0-dev in the Dockerfile makes docker build fail: The following packages have unmet dependencies: libmysqlclient-dev : Depends: libssl-dev (>= 1.1.1-1ubuntu2.1~18.04.5~) but it is not going to be installed E: Unable to correct problems, you have held broken packages. There seems to be a conflict with different versions of libssl and libmysqlclient that did not exist previously with Ubuntu 18.04. Just use the current libssl-dev from Ubuntu 18.04 to fix the build. Signed-off-by: Pranav Annam <pranavannam@gmail.com> [rephrased commit message] Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
* docs: set the GID in the .env filePranav Annam2020-02-17
| | | | | | | | | | | | | | | The .env file needs the UID and GID for a working docker-compose build. If you follow the current instructions, the docker build fails with a clear error message: 'You must define GID in .env' It is still good to update documentation to reduce the burden on new contributors to run into this build failure first. Signed-off-by: Pranav Annam <pranavannam@gmail.com> [simply provide bash command in docs, reworked commit message] Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
* REST: Fix duplicate project queriesMete Polat2020-02-02
| | | | | | | | | | | Eliminates duplicate project queries caused by calling get_absolute_url() in the embedded serializers. Following foreign keys with 'series__project' will cache the project of the series as well as the series itself. Signed-off-by: Mete Polat <metepolat2000@gmail.com> Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: #335
* Add regression tests for bug #335Stephen Finucane2020-02-02
| | | | | | Signed-off-by: Stephen Finucane <stephen@that.guru> Cc: Mete Polat <metepolat2000@gmail.com> Related: #335
* Handle pull requests with random trailing spaceKonstantin Ryabitsev2020-02-01
| | | | | | | | | | | | Another fix for copy-pasted pull requests, this time for cases when something is copy-pasted from a terminal and retains all the bogus trailing whitespace. Example: https://lore.kernel.org/r/043eb5b2-a302-4de6-a3e8-8238e49483b1@ti.com Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
* Patchwork v2.2.0-rc1Stephen Finucane2019-12-27
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Bump API version in docs to 1.2Stephen Finucane2019-12-27
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* templates: Combine series and related rowMete Polat2019-12-27
| | | | | | | | | Move the series patch list from row 'Related' to 'Series'. This allows us to use the 'Related' row for actually showing submission relations instead. Signed-off-by: Mete Polat <metepolat2000@gmail.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* fixtures: Fix typo in default_statesStephen Finucane2019-12-27
| | | | | Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 42b1dfa2 ("models: Add State.slug field")
* tests: Add test for broken parserStephen Finucane2019-12-27
| | | | | | | | | | Add tests for the recent changes we made to how we parse multiple series received at once. These tests actually highlighted what appeared to be the test failure that's been intermittently breaking our CI for years now, so the 'expectedFailure' marker has been removed in the hope that this is actually the case. Signed-off-by: Stephen Finucane <stephen@that.guru>
* parser: Use a second query to weed out duplicate seriesStephen Finucane2019-12-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Annoyingly, not all email clients properly thread emails using the message ID fields originally specified in RFC 822 [1]. Worse, some MTAs (cough, outlook.com, cough) actually override what the client configures, breaking the world in the process. Realising this is an issue, Patchwork supports threading using arbitrary metadata in addition to the RFC 822 metadata. Specifically, it uses a combination of submitter and list-id extracted from the headers along with the series version and total count metadata extracted from the subject. In addition to this, we timebox things so that two or more series that match on all of this metadata but which are sent some time apart from each other aren't combined by accident. This does leave one edge case - duplicate series received within the timebox will be combined. We've resigned ourselves to this fact on the basis that it's extremely unlikely for all of these things to go wrong at once. Given all the above, there should be no reason that attempting to find series by series markers should return more than one series. The timeboxing will prevent us grouping similar looking series by accident and the only other reason for this to happen is because we lost a race and we should try again. [1] https://tools.ietf.org/html/rfc822 Signed-off-by: Stephen Finucane <stephen@that.guru> Cc: Daniel Axtens <dja@axtens.net>
* models: Use database constraints to prevent split SeriesStephen Finucane2019-12-27
| | | | | | | | | | | | | | | | | | | | | | | | | Currently, the 'SeriesReference' object has a unique constraint on the two fields it has, 'series', which is a foreign key to 'Series', and 'msgid'. This is the wrong constraint. What we actually want to enforce is that a patch, cover letter or comment is referenced by a single series, or rather a single series per project the submission appears on. As such, we should be enforcing uniqueness on the msgid and the project that the patch, cover letter or comment belongs to. This requires adding a new field to the object, 'project', since it's not possible to do something like the following: unique_together = [('msgid', 'series__project')] This is detailed here [1]. In addition, the migration needs a precursor migration step to merge any broken series. [1] https://stackoverflow.com/a/4440189/613428 Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: #241 Cc: Daniel Axtens <dja@axtens.net> Cc: Petr Vorel <petr.vorel@gmail.com>
* tests: Skip tests that fail on SQLite DB backendStephen Finucane2019-12-27
| | | | | | | | These are failing due to differences in behavior of the backend. Since this will never be used for production, we can simply skip these unit tests and rely on the CI to catch potential issues. Signed-off-by: Stephen Finucane <stephen@that.guru>
* settings: Add configuration for sqlite DB backendStephen Finucane2019-12-27
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* models: Add State.slug fieldStephen Finucane2019-12-27
| | | | | | | | | | | | | This reduces a lot of the tech debt we had built up around this. This field is populated from a slugified representation of State.name and has a uniqueness constraint. As a result, we also need to a uniqueness constraint to the State.name field. This shouldn't be an issue and probably should have been used from day one. Note that there is no REST API impact for this since we've been using state slugs all along. Signed-off-by: Stephen Finucane <stephen@that.guru>
* REST: Allow configuration of user settings via APIStephen Finucane2019-12-24
| | | | | | Expose the embedded UserProfile field via the REST API. Signed-off-by: Stephen Finucane <stephen@that.guru>
* tests: Provide a way to disable API schemaStephen Finucane2019-12-24
| | | | | | | | | | | | | The API schema validation is strict, in that it will error out with invalid keys in either the request or response. Unfortunately the API itself is not. We're hopefully going to fix this in a distant v2.0, but for now we need a way to ensure that the API does what it's supposed to, namely not set fields that don't exist or that the user isn't allowed to set, even if proper error codes aren't raised. This isn't actually used yet. That will come later. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Add missing series index schemaMete Polat2019-12-08
| | | | | | Fixes: 7d8e24bc84bd ("docs: Start documenting API using OpenAPI") Signed-off-by: Mete Polat <metepolat2000@gmail.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* requirements: Add pyup markers to prevent dumb PRsStephen Finucane2019-12-01
| | | | | | | | | | | Until [1] is merged, we're going to have to override what these markers are doing. Perhaps it would be easier to just specify the markers in the comments as the actual marker, but I like using pip's features and the comments *should* be temporary. [1] https://github.com/pyupio/pyup/pull/367 Signed-off-by: Stephen Finucane <stephen@that.guru>