Daniel Axtens [Tue, 17 Mar 2020 13:59:16 +0000 (00:59 +1100)]
REST: Add release note for faster queries
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>
(cherry picked from commit 271d91341e0e9ebea13fcfc46fb2f68106753c66) Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Tue, 17 Mar 2020 13:59:15 +0000 (00:59 +1100)]
REST: extend performance improvements to other parts of the API
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>
(backported from commit 046aa155c3bf827691bab9e1df8916c969a30d54
- dropped tests, it depends on a test we don't carry
- rejigged to suit old M:N series model and old API) Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Tue, 17 Mar 2020 13:59:14 +0000 (00:59 +1100)]
REST: fix patch listing query
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>
(backported from commit 98a2d051372dcedb889c4cb94ebd8ed7b399b522
- dropped tests, it depends on a test we don't carry
- rejigged to suit old M:N series model) Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Tue, 17 Mar 2020 13:59:13 +0000 (00:59 +1100)]
REST: massively improve the patch counting query under filters
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>
(backported from commit 97155c0bc8881787f6c536031b678a4c3f89bda6
old django-filters uses 'name' instead of 'field_name') Signed-off-by: Daniel Axtens <dja@axtens.net>
Mete Polat [Wed, 29 Jan 2020 19:01:22 +0000 (20:01 +0100)]
REST: Fix duplicate project queries
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
(backported from commit ec00daae4d79bf2560034e1b2bc3cf76a98a3212
dropped all the tests, they clash horribly) Signed-off-by: Daniel Axtens <dja@axtens.net>
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.
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 1633afe5b46042d522777f66b1959a82298d0ab2)
When git-request-pull output is pasted into a mail client instead of
mailed directly, the ref part of the pull URL may end up wrapped to the
next line.
This change properly parses URLs both with and without newlines.
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
(cherry picked from commit 8c229caa71d58e971708ac7ebdb02d6858cd2a4c)
Ali Alnubani [Mon, 21 Oct 2019 15:06:25 +0000 (15:06 +0000)]
docs: Fix link to deployment guide
The old format redirects to a nonexistent page when
there are multiple versions of the docs.
Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit 01ae4d15c628038534792f9758152e87820d5e57)
Andrew Donnellan [Mon, 21 Oct 2019 07:37:31 +0000 (18:37 +1100)]
templates: Fix mismatched close tags
There's a </td> rather than </th> in the bundle list. Fix it.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit 005ba0644dab1f386adaf4e603422091bb31cdca)
The patch adds missing commas in the table lists where missing, and
removes where unnecessary. This fixes errors such as the following when
feeding the script to psql:
psql:lib/sql/grant-all.postgres.sql:37: ERROR: syntax error at or near "patchwork_emailconfirmation"
LINE 19: patchwork_emailconfirmation,
...
Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Reviewed-by: Stephen Finucane <stephen@that.guru> Fixes: ca0e79d4db34 ("sql: Sort 'grant-all' scripts alphabetically") Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Sat, 21 Sep 2019 18:06:14 +0000 (19:06 +0100)]
Fix issue with delegation of patch via REST API
There have been reports of people being unable to delegate patches to
themselves, despite being a maintainer or the project to which the patch
is associated.
The issue is a result of how we do a check for whether the user is a
maintainer of the patch's project [1]. This check is checking if a given
'User.id' is in the list of items referenced by
'Project.maintainer_project'. However, 'Project.maintainer_project' is a
backref to 'UserProfile.maintainer_projects'. This means we're comparing
'User.id' and 'UserProfile.id'. Boo.
This wasn't seen in testing since we've had a post-save callback [2] for some
time that ensures we always create a 'UserProfile' object whenever we create a
'User' object. This also means we won't have an issue on deployments initially
deployed after that post-save callback was added, a 'User' with id=N will
always have a corresponding 'UserProfile' with id=N. However, that's not true
for older deployments such as the ozlabs.org one.
Stephen Finucane [Sun, 28 Oct 2018 17:14:45 +0000 (17:14 +0000)]
tests: Split up tests in 'test_patch'
This is essentially commit ad54f526869804abcdcea8744d90ae85bd0ce34c
("tests: Add 'store_samples' decorator to 'test_patch'") from master
without the 'store_samples' decorator. It's included to avoid merge
conflicts with future patches.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stable-Only
Daniel Axtens [Wed, 18 Sep 2019 06:17:28 +0000 (16:17 +1000)]
parsearchive, mail: use repr() to get a human readable exception
Currently if we have particular types of error in mail parsing
in parsearchive or parsemail, we print exc.message, which doesn't
always work:
Traceback (most recent call last):
File ".../patchwork/management/commands/parsearchive.py", line 90, in handle
obj = parse_mail(msg, options['list_id'])
File ".../patchwork/parser.py", line 961, in parse_mail
raise ValueError("Missing 'Message-Id' header")
ValueError: Missing 'Message-Id' header
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
File ".../django/core/management/__init__.py", line 381, in execute_from_command_line
utility.execute()
File ".../django/core/management/__init__.py", line 375, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File ".../django/core/management/base.py", line 323, in run_from_argv
self.execute(*args, **cmd_options)
File ".../django/core/management/base.py", line 364, in execute
output = self.handle(*args, **options)
File ".../patchwork/management/commands/parsearchive.py", line 100, in handle
logger.warning('Invalid mail: %s', exc.message)
AttributeError: 'ValueError' object has no attribute 'message'
repr(exc) will work. Use it.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 319b6f9d750c220d47c1044246d4e97fd500c6d4)
Daniel Axtens [Wed, 21 Aug 2019 05:15:44 +0000 (15:15 +1000)]
mbox: do not copy Content-Type into exported mbox
Daniel reports a patch + comment combination that breaks in
git am. The patch reports a Content-Type of US-ASCII, while
the comment adds a Ack with UTF-8 characters. The exported
mbox contains both the original Content-Type, and a UTF-8
Content-Type that we set. However, because the US-ASCII one
occurs later, git am honours it instead of ours, and chokes
on the UTF-8 characters.
Strip out any subsequent Content-Type:s. We normalise things
to UTF-8 and should not allow it to be overridden.
Add a test for this, based on the original report.
Reported-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
filters: Escape State names when generating selector HTML
States with names containing special characters are not correctly escaped
when generating the select list. Use escape() to fix this.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
(cherry picked from commit b3fa0c402e060622a5ed539a465d2fa98b1d2e13) Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Fri, 5 Jul 2019 01:30:28 +0000 (11:30 +1000)]
templatetags: Do not mark output of msgid tag as safe
The msgid template tag exists to remove angle brackets from either side of
the Message-ID header.
It also marks its output as safe, meaning it does not get autoescaped by
Django templating.
Its output is not safe. A maliciously crafted email can include HTML tags
inside the Message-ID header, and as long as the angle brackets are not at
the start and end of the header, we will quite happily render them.
Rather than using mark_safe(), use escape() to explicitly escape the
Message-ID.
Stephen Finucane [Fri, 21 Sep 2018 16:59:48 +0000 (17:59 +0100)]
settings: Don't configure logging for parsearchive
A recent change added additional ERROR level logs to the 'parsearchive'
tool. This highlighted an issue, whereby we had configured all modules
in 'patchwork.management.command' to email administrators on ERROR logs.
We clearly shouldn't be doing this for the 'parsearchive' command or for
anything other than 'parsemail', so fix this.
Along the way, we remove a now-unnecessary 'extra' argument to one of
the logging calls in 'parsearchive' and resolve a pep8 issue.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 133091da ("parsearchive: Fix logging") Cc: Daniel Axtens <dja@axtens.net>
(cherry picked from commit aa266a28c6a022ad6952fac8f36afff0c540dccf)
Stephen Finucane [Wed, 19 Sep 2018 20:03:33 +0000 (21:03 +0100)]
parser: Handle IntegrityError for cover letters, comments
This was already done for patches but cover letters and comments were
not handled correctly, resulting in errors while parsing archives. While
we're here, we slightly modify how these exceptions are handle. Rather
than simply ignoring them, as we were doing, we raise a custom
exception. This allows us to specifically identify these types of
exceptions, print a log and still skip them (which we want, as seen in
commit d2eb1f6d2).
While we're here, we change from separate create-save calls to a
combined create-save call for both created CoverLetter and Comment
objects. We were already doing this for patches.
Petr Vorel [Tue, 4 Jun 2019 15:56:39 +0000 (17:56 +0200)]
parser: Remove duplicity
commit fc1d750 copied lines added in 753e457.
Make sense to define it on single place (DRY).
Signed-off-by: Petr Vorel <petr.vorel@gmail.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit ecbe3fc5f1c91cfa19dab77d527897e2122f5096)
Commit 753e4572d updated the parser to consider additional header lines
when deciding where a patch message ends and the diff begins. However,
these additional lines were not captured meaning these patches didn't
have a diff associated with them and they therefore weren't patches in
the Patchwork sense of the term. Correct this and add a test.
Petr Vorel [Sun, 5 May 2019 20:20:05 +0000 (22:20 +0200)]
parser: Add missing extended header lines
Patchwork didn't recognise some patches due missing some extended header
lines (e.g. "old mode" and "new mode" for renaming file mode, see [1]).
Thus adding all modes from git doc [2].
In July 2018, we received a report of OzLabs patchwork mangling
emails that have subjects containing words with internal commas,
like "Insert DT binding for foo,bar" (#197).
Stephen took a look and came up with the comment this reverts. Quoting
the commit message:
RFC2822 states that long headers can be wrapped using CRLF followed by
WSP [1]. For example:
Subject: Foo bar,
baz
Should be parsed as:
Foo bar,baz
As it turns out, this is not the case. Journey with me to
section 2.2.3 of RFC 2822:
2.2.3. Long Header Fields
Each header field is logically a single line of characters comprising
the field name, the colon, and the field body. For convenience
however, and to deal with the 998/78 character limitations per line,
the field body portion of a header field can be split into a multiple
line representation; this is called "folding". The general rule is
that wherever this standard allows for folding white space (not
simply WSP characters), a CRLF may be inserted before any WSP. For
example, the header field:
Subject: This is a test
can be represented as:
Subject: This
is a test
So the issue with the example in the reverted commit is that there is no
folding white space in "bar,baz", so it's not valid to split it.
These are valid:
Subject: Foo bar,baz
Subject: Foo
bar,baz
but splitting "bar,baz" into "bar,\n baz" is not valid.
What then is correct unfolding behaviour? Quoting the RFC again:
The process of moving from this folded multiple-line representation
of a header field to its single line representation is called
"unfolding". Unfolding is accomplished by simply removing any CRLF
that is immediately followed by WSP. Each header field should be
treated in its unfolded form for further syntactic and semantic
evaluation.
In other words, the unfolding rule requires you to strip the CRLF, but
it does not permit you to strip the WSP. Indeed, if "bar,\n baz" is
received, the correct unfolding is "bar, baz".
If you do strip the WSP, you end up mashing words together, such as in
https://patchwork.ozlabs.org/patch/1097852/
So revert the commit, restoring original behaviour, but keep a corrected
version of the test.
This presents a big question though: how did Rob's email up with a
mangled subject line?
To answer this question, you end up having to learn about OzLabs
Patchwork and how it differs from Patchwork the project.
OzLabs Patchwork (patchwork.ozlabs.org) is an installation of Patchwork.
Part of what makes it so useful for so many projects is a little
intervening layer that can massage some mail to make it end up in the
right project. Email that lands in the device tree project is an example
of email that goes through this process. I only learned about this
today and I haven't looked in any detail at precisely what is done to
the mail. The script is not part of the Patchwork project.
This intervening filter is a Python script that runs - and this is an
important detail - in Python 2.7.
Ignoring all the details, the filter basically operates in a pipe
between the mail program and patchwork's parsemail, like
(mail from system) | filter.py | parsemail
At it's very simplest, filter.py acts as follows:
import email
import sys
mail = email.parse_from_file(sys.stdin)
sys.stdout.write(mail.as_string())
Fascinatingly, if you take Rob's email from #197 and put it through this
process, you can see that it is getting mangled:
You can see that python27 has incorrectly wrapped the header, breaking
where there is not a foldable space. Python3 does not have this issue.
To summarise:
- part of the magic of OzLabs PW is a filter to make sure mail gets to
the right place. This isn't part of the Patchwork project and so is
usually invisible to patchwork developers.
- the filter is written in python27. The email module in py27 has a bug
that incorrectly breaks subjects around commas within words.
- patchwork correctly unfolds those broken subjects with a space after
the comma.
- the extra space was interpreted as a bug in patchwork, leading to a
misinterpretation of the spec to strip out the whitespace that was
believed to be in error.
- that broke other wrapped subjects.
To solve this, revert the commit and I'll work with jk to get the filter
script into py3 compatibility. (Given that py27 sunsets in ~7mo, trying
to fix it is not worth it.)
Closes: #273 Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru>
[stephenfin: Use a new release note instead of editing the original one]
(cherry picked from commit 0c7d45e35fe409dd056ccc4558808a8e97ffaf0e)
Stephen Finucane [Thu, 18 Oct 2018 16:45:21 +0000 (17:45 +0100)]
Add support for Django REST Framework 3.9
As with 3.7 and 3.8, there are no breaking changes we need to be
concerned with here.
Signed-off-by: Stephen Finucane <stephen@that.guru>
(backported from commit fab5571e2f20323feed84609a52778a0f8e41e32
to not include changes to requirements-*.txt, just release note
and tox.ini changes.) Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Sun, 24 Jun 2018 19:55:49 +0000 (20:55 +0100)]
Add support for django-filter 1.1
There's one warning to handle here.
Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
(backported from commit 4ad2558f884bf33201e179a171ad3821a62126a1
to not include changes to requirements-*.txt, just release note,
code and tox.ini changes.) Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Sun, 24 Jun 2018 19:55:48 +0000 (20:55 +0100)]
Add support for Django REST Framework 3.7, 3.8
No breaking changes that concern us here.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
(backported from commit a27f36fb784867e434527b8e65064ae3bdb12c82
to not include changes to requirements-*.txt, just release note
and tox.ini changes.) Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Tue, 30 Apr 2019 06:03:04 +0000 (16:03 +1000)]
REST: A check must specify a state
The Ozlabs crew noticed that a check without a state caused a
KeyError in data['state']. Mark state as mandatory, check for
it, and add a test.
NOTE(daxtens): Swagger changes are excluded from the backport
Reported-by: Russell Currey <ruscur@russell.cc> Reported-by: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit 7a20ccda99e48dab643d1fbd7e170fe3e4c47185)
Daniel Axtens [Tue, 30 Apr 2019 06:03:03 +0000 (16:03 +1000)]
REST: Handle regular form data requests for checks
08d1459a4a40 ("Add REST API validation using OpenAPI schema") moved
all API requests to JSON blobs rather than form data.
dc48fbce99ef ("REST: Handle JSON requests") attempted to change the
check serialiser to handle this. However, because both a JSON dict
and a QueryDict satisfy isinstance(data, dict), everything was handled
as JSON and the old style requests were broken.
Found in the process of debugging issues from the OzLabs PW & Snowpatch
crew - I'm not sure if they actually hit this one, but kudos to them
anyway as we wouldn't have found it without them.
NOTE(daxtens): This does not need the new tests as we do not have 08d1459a4a40, so we just need the fix to the API. We do not add a JSON
test to stable.
Fixes: dc48fbce99ef ("REST: Handle JSON requests") Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit 666de29ebada5990a8d69f4d71d6bb271e1a68c3)
Jeremy Kerr [Tue, 30 Apr 2019 06:03:01 +0000 (16:03 +1000)]
notifications: fix notification expiry when no user is associated
It's possible that an EmailConfirmation object will have no associated
user (eg, for email opt-out, which does not require a user object). In
this case, we will see a NULL value for EmailConfirmation.user_id.
However, having a NULL value appear in a SQL 'IN' clause will match
every value. This means that once one of these null-user
EmailConfirmations is present, we will never expire any non-active user
accounts.
This change adds a filter for a valid user_id when we query for active
EmailConfirmation objects. This means we'll have a valid values set to
use in the pending_confs set.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
[dja: fix pep8 issue] Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit d0b79d9dee04aee13c8d64a193a7818f72eeca3b)
Andrew Donnellan [Tue, 30 Apr 2019 06:03:00 +0000 (16:03 +1000)]
REST: Add new setting for maximum API page size
In 41790caf59ad ("REST: Limit max page size") we limited the maximum page
size to the default page size in the settings.
This turns out to be rather restrictive, as we usually want to keep the
default page size low, but an administrator may want to allow API clients
to fetch more than that per request.
Add a new setting, MAX_REST_RESULTS_PER_PAGE, to set the maximum page size.
Closes: #202 ("Separate max API page size and default API page size into different settings") Suggested-by: Stewart Smith <stewart@linux.ibm.com> Suggested-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
[dja: set to 250 as per mailing list discussion] Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit 8fe11180a1a59f6e8e5a4441b21a3d9831f0b69d)
Stephen Finucane [Tue, 30 Apr 2019 06:02:59 +0000 (16:02 +1000)]
REST: Check.user is not read-only
We only support 'Check' creation - not check updating. As a result,
there's no real reason that the 'Check.user' field should be read-only
and this is causing an issue with Django REST Framework 3.7. Simply
remove the attribute and extend the tests to validate things are working
as expected.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit c9cc59dac70d76971a5342ca53e2b13eb93592de)
Daniel Axtens [Thu, 28 Feb 2019 04:29:53 +0000 (15:29 +1100)]
parser: recognise git commit consisting only of empty new file
Commits with only an empty new file are liable to be missed.
The parser state machine doesn't recognise the headers "new
file mode" and "index": teach it about them.
Add a test to demonstrate.
It's a little bit academic as you don't usually send patches like
that but sometimes you do, especially if you're a snowpatch dev :)
Closes: #256 Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 9dd0b5ba843ccac5cf768a1c34226abe8b85cf6d)
Stephen Finucane [Sat, 22 Dec 2018 22:54:05 +0000 (22:54 +0000)]
tests: Add tests for 'series-completed' event
In commit 087fe50, we resolved an issue with 'series-completed' events.
That entire fix doesn't apply here due to commit 76505e9, but the tests
can be included to ensure we don't ever regress.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stable-Only
Django Admin seems to be doing something funky with how it's handling
the creation of a User's corresponding UserProfile instance when
modelled as an inline field. Re-setting the UserProfile.user attribute
seems to resolve the issue, so do just that.
Stephen Finucane [Fri, 26 Oct 2018 20:35:03 +0000 (21:35 +0100)]
REST: Show 'web_url' in embedded series responses
Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: #224 Fixes: 9c179bf4c ("REST: Add 'web_url' link to API responses")
(cherry picked from commit ee58fb3be6eaa0deeae966ef3b95fc757df4519b)
Stephen Finucane [Sun, 24 Jun 2018 19:55:55 +0000 (20:55 +0100)]
Resolve Python 3.6 warnings
DeprecationWarning: invalid escape sequence \d
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit 67c76281c25efd88dfda1a86f4b277e8f1bfbb53)
Stephen Finucane [Fri, 26 Oct 2018 09:39:29 +0000 (10:39 +0100)]
tox: Disable W504 ("line break after binary operator")
This was introduced in a recent version of 'pycodestyle'. The
documentation notes [1] that it is mutually exclusive with W503, which
we do enforce, suggesting that we disable one or the other. Avoid the
churn and stick to the older rule, which I personally prefer.
NOTE(stephenfin): This isn't technically required here, but it does help
us maintain a passing 'pep8' tox target on this branch which is
generally useful for catching things like SyntaxErrors.
Stephen Finucane [Thu, 11 Oct 2018 10:47:56 +0000 (11:47 +0100)]
REST: Allow setting of values using embedded serializers
Unfortunately, the use of embedded serializers for some fields breaks
the ability to update these fields, either via the HTML interface (where
the widget is totally busted) or via a client like 'git-pw'. What we
actually want is to be able to update these fields like normal primary
key but show them using the embedded serializer. We do just this by
using a modified variant of the PrimaryKeyRelatedField and using the
serializers simply for displaying.
Stephen Finucane [Thu, 11 Oct 2018 13:53:25 +0000 (14:53 +0100)]
REST: Validate patch delegate
At present, only users who are maintainers of projects can be delegated
a project. Validate this. This is currently broken due to #216 but that
will be fixed in a future change.
Jiri Benc [Thu, 28 Jun 2018 19:42:11 +0000 (15:42 -0400)]
parser: fix parsing of patches with headings
Some people tend to use lines full of '=' as a fancy way to format headings
in their commit messages in a rst-like style. However, the current parser
treats such lines as a beginning of a diff.
The only currently used tool that produces diffs with '=' lines is quilt in
the default configuration. However, even with quilt, the diff looks this
way:
It's enough to match on the "Index:" line. The state of the state machine is
kept at 1 when it encounters the '=' line, thus it's safe to remove the
match on '=' completely.
[This prevents us from properly parsing metadata out of the changelog. -dcz ]
Signed-off-by: Jiri Benc <jbenc@redhat.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 67faf96ab96d93252c89967ef766bcbe8214c0fc)
Petr Vorel [Thu, 26 Jul 2018 07:11:14 +0000 (09:11 +0200)]
pwclient: Fix pwclient am output formatting
repr() print unicode prefix for string:
$ pwclient git-am N
Applying patch #N using u'git am'
Remove it:
$ pwclient git-am N
Applying patch #918868 using "git am"
git mixes single and double quotes, use double quotes which are more
frequently used.
Signed-off-by: Petr Vorel <petr.vorel@gmail.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit ea5847ada3ac79908a2b251839e77e1b2f3dc6d2)
Stephen Finucane [Sat, 29 Sep 2018 20:11:22 +0000 (21:11 +0100)]
filters: Pre-populate delegate, submitter filters
This appears to have got lost in the transition to 'selectize.js'. In
brief, this will ensure that a previously selected delegate or submitter
is still enabled post-filtering. For more information, see [1].
We had registered an event handler on a checkbox in table header which
would call a function, 'checkboxes', on all checkboxes within that
table. This function, in turn, causes does its work and then triggers
event handlers for all modified checkboxes which include the original
table header checkbox. This resulted in the original event calling
itself recursively.
Resolve this by only modifying the checkboxes in the table body.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 44fe7bae ("js: Allow shift-select of checkboxes")
(cherry picked from commit ae154148c78a75ff73c3c22f0ff0c6b3a3d01408)
Yuri Volchkov [Wed, 20 Jun 2018 12:21:42 +0000 (14:21 +0200)]
parsemail: ignore html part of multi-part comments
Currently an html-protection present only for patch-emails. If a
multi-part comment-email arrives, it messes up patchwork. In my case,
the symptom was a non intended 'Signed-off-by' in the downloaded
patches, with html-like junk.
This patch makes parsemail skip all parts of comment which are not
text/plain.
Of course, this will drop html-only emails completely. But they can
not be parsed anyways.
Signed-off-by: Yuri Volchkov <yuri.volchkov@gmail.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit e27ff061dc01e51967a978884a5c59152863ab9c)
This provides an easy way for clients to navigate to the web view. The
URL is added to four resources: bundles, comments, cover letters and
series. We could also extend this to projects and users in the future,
but the latter would require renaming an existing property while the
latter would require a public "user" page which does not currently
exists.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Wed, 13 Jun 2018 21:53:22 +0000 (22:53 +0100)]
Don't discard values from 'archive' filter
The pagination functionality used in the 'patchwork.view.generic_list'
generates the filter querystring from scratch. To do this, it calls the
'patchwork.filters.Filters.params' function, which in turn calls the
'patchwork.filters.Filter.key' function for each filter. If any of these
'key' functions return None, the relevant filter is not included in the
querystring. This ensures we don't end up with a load of filters like
the below:
There is one exception to this rule, however: ArchiveFilter. This is a
little unusual in that it is active by default, excluding patches that
are "archived" from the list. As a result, the 'key' function should
return None for this active state, not for the disabled state. This has
been the case up until commit d848f046 which falsely equated 'is False'
with 'is None'. This small typo resulted in the filter being ignored
when generating pagination links and essentially broke pagination for
some use cases.
Fix this up. We could probably simplify this thing greatly by not
recalculating filters for pagination at least or, better yet, by using
django-filter here too. That is a change for another day though.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reported-by: John McNamara <john.mcnamara@intel.com> Reported-by: Eli Schwartz <eschwartz93@gmail.com> Fixes: d848f046 ("trivial: Don't shadow built-ins") Closes: #184
views: Raise 404 if downloading non-existent dependencies
If a patch was processed by Patchwork before series support was added,
it will not have a series associated with it. As a result, it is not
possible to extract the dependencies for that patch from the series and
a 404 should be raised. This was not previously handled correctly.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net> Reported-by: John McNamara <john.mcnamara@intel.com> Fixes: e2dfd490 ("views: Add 'series' parameter to '/mbox' endpoint") Closes: #189
docs: Update deployment installation guide for v2.1
There are no actual changes needed from the Patchwork side so this is
mostly a cleanup.
- Use Ubuntu 18.04 (including package names)
- Resolve some minor issues with commands
- Remove use of "trust" authentication for PostgreSQL
- Minor style changes
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Wed, 30 May 2018 10:57:59 +0000 (11:57 +0100)]
sql: Update 'grant-all.mysql' script with missing tables
These were all introduced in 2.0 and while the postgreSQL script was
fixed in commit 234bc7c3, the MySQL one was not. This suggests either
(a) no one is using this or (b) people are carrying local changes but
for now we just resolve the issues.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Fri, 11 May 2018 17:00:44 +0000 (03:00 +1000)]
REST: Disable control for filtering patches by series in web view
As with the events view, creating and rendering the control for
filtering patches by series creates a massive slowdown. It's a little
sad not to be able to do this in the web UI as filtering patches
by series does make sense, but hopefully people figure out you can
still do it, just not from the web view.
Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru>
docs: Add additional information about API versions
As we're soon going to be supporting a v1.1 API, we should document what
versions are available and whether they're supported still (hint: we
support both v1.0 and v1.1 at present).
Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
The root cause of this performance issue was not the use of the
JSONRenderer but rather the population of filter forms. The latter is
now disabled, meaning we can start using the original renderer.
Signed-off-by: Stephen Finucane <stephen@that.guru> Cc: Daniel Axtens <dja@axtens.net> Acked-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Thu, 10 May 2018 14:45:05 +0000 (15:45 +0100)]
REST: Resolve performance issues with '/events' web view
The dropdown select-based filters in the web view of the REST API have
stung us a few times. In this case, populating these filters for the
'/events' endpoint results in a huge query that hammers the database and
results in seriously laggy responses.
The root cause of this performance issues was erroneously identified as
an issue with the JSON renderer so that particular patch can now be
reverted. This will be done separately.
Signed-off-by: Stephen Finucane <stephen@that.guru> Cc: Daniel Axtens <dja@axtens.net> Tested-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Wed, 11 Apr 2018 16:13:38 +0000 (17:13 +0100)]
REST: Use DRF-specific filterset
This variant of 'FilterSet' will convert a 'django.forms.ValidationError',
which wouldn't be handled by DRF, to a
'rest_framework.exceptions.ValidationError', which would be.
Signed-off-by: Stephen Finucane <stephen@that.guru>
[dja: commit message] Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Wed, 11 Apr 2018 16:13:36 +0000 (17:13 +0100)]
REST: Rename Filter -> FilterSet
FilterSets are to Forms as Filters are to Fields: the former is
made up of the latter. We have a FilterSet for each resource that we
wish to support filtering on (i.e. all of them).
Rename our "Filters" to the more appropriate FilterSets.
The old name was confusing and will conflict with some forthcoming changes.
Signed-off-by: Stephen Finucane <stephen@that.guru>
[dja: commit message] Signed-off-by: Daniel Axtens <dja@axtens.net>
projects / thirdparty / patchwork.git / log
