| Commit message (Collapse) | Author | Age |
... | |
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
| |
This is what users will want right now.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
| |
This is unnecessary and was disabling reno's built-in ability to detect
a base branch.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
| |
Include migrations in the flake8 checks, which should catch some simple
undefined variables errors.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Daniel Axtens <dja@axtens.net>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
Related: #343
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
| |
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")
|
|
|
|
|
|
|
|
| |
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")
|
|
|
|
|
|
| |
There are no breaking changes apparent.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
| |
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
| |
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")
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
Cc: Mete Polat <metepolat2000@gmail.com>
Related: #335
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
Fixes: 42b1dfa2 ("models: Add State.slug field")
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
| |
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
| |
Expose the embedded UserProfile field via the REST API.
Signed-off-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
|
|
| |
Fixes: 7d8e24bc84bd ("docs: Start documenting API using OpenAPI")
Signed-off-by: Mete Polat <metepolat2000@gmail.com>
Reviewed-by: Stephen Finucane <stephen@that.guru>
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|