aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* tests: Remove Selenium testsStephen Finucane2018-04-26
| | | | | | | | | | | | | | | | These were added quite some time ago in order to allow some level of UI testing. However, I've personally never used them, they're not used by the CI, and no one has shown any desire in extending them in their time here. It is time to bid these tests adieu. Removing these allows us to remove a whole load of wiring that existed just to enable these. Some of this, like the '--quick-tox' option for the Dockerfile, is retained so we don't need to use different commands for various versions of Patchwork, but the majority is just stripped out. Signed-off-by: Stephen Finucane <stephen@that.guru> Cc: Daniel Axtens <dja@axtens.net>
* docker: Replace tabs with spacesStephen Finucane2018-04-26
| | | | | | Maintain your chill, people. Signed-off-by: Stephen Finucane <stephen@that.guru>
* Update django-debug-toolbar from 1.8 to 1.9.1pyup-bot2018-04-26
|
* Update sqlparse from 0.2.3 to 0.2.4pyup-bot2018-04-25
|
* Update sphinx_rtd_theme from 0.2.4 to 0.3.0pyup-bot2018-04-25
|
* Resolve pep8 issuesStephen Finucane2018-04-25
| | | | | Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 26f22a87 ("models: Add a backreference for a user's bundles")
* models: Add a backreference for a user's bundlesStephen Finucane2018-04-25
| | | | | | This is more intuitive. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Add release note for changes to 'headers' fieldStephen Finucane2018-04-25
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* api: Show all headers with the same keyVeronika Kabatova2018-04-25
| | | | | | | | | | | | | | While the code on our side returns all (key, value) pairs for email headers, Django's REST framework probably uses dictionaries behind the scenes. This means that having multiple headers with same key (eg 'Received', which is totally valid and common situation), only one of these headers is visible in the REST API. Let's hack around this by returning a list of values in case the key is present multiple times. Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* docs: Random fixesStephen Finucane2018-04-24
| | | | | | | Remove an unnecessary 'toctree' from the index page and fix some definition lists. Signed-off-by: Stephen Finucane <stephen@that.guru>
* tests: Replace incorrect testsStephen Finucane2018-04-22
| | | | | | | | | | | | In commit 683792d1, the 'test_api.py' was split into multiple 'api/test_xyz.py' files. As part of this change, the tests for cover letter were mistakenly included in place of tests for checks. Correct this oversight. Signed-off-by: Stephen Finucane <stephen@that.guru> Reported-by: Veronika Kabatova <vkabatov@redhat.com> Acked-by: Veronika Kabatova <vkabatov@redhat.com> Fixes: 683792d1 ("tests: Split 'test_rest_api'")
* gitignore: Ignore '.venv'Stephen Finucane2018-04-14
| | | | | | | | I use this to validate stuff quite frequently. Ignore it so it's not polluting my git-status. Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* tox: Add 'docs' to default environmentsStephen Finucane2018-04-14
| | | | | | | | | I'd simply run 'tox' (via docker) to validate some previous patches. Sadly that didn't catch a release note issue. Make sure this doesn't happen again by always running 'docs'. Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* docs: Fix a release note issueStephen Finucane2018-04-14
| | | | | | | | | | | | | | You can't have a space before the closing double backticks of a literal block. This was generating the following (unhelpful) error message: patchwork/docs/releases/unreleased.rst:82:Inline literal start-string without end-string. reno should have better error messages but that's a problem for another day. Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* docs: Read version from 'patchwork.VERSION'Stephen Finucane2018-04-14
| | | | | | | | Because this isn't an installable package we need to do some path hackery. Not the end of the world though. Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* Fix stuff around mbox header changesVeronika Kabatova2018-04-11
| | | | | | | | | | | | | | | | | | | | Bundle tests got broken after the subject in mbox was changed from the parsed version to the original one because the tests checked for the presence of patch's name in the response. Fixing this turned out to be a bit tricky since the tests check the mbox attachment and HTML responses separately, so we need a string that would be present in both (the intuitive idea of checking X-Patchwork-Id won't work well). Add the patch's name to the content of the test patch so we can continue testing things the same way, checking for the presence of patch's name. Also add a releasenote notifying about the inclusion of the original headers. Reverts: b2a25342 ("Use parsed subject for mboxes") Fixes: 01b9cbb9 ("Include all email headers in mboxes") Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* gitignore: Ignore sql filesStephen Finucane2018-04-09
| | | | | | These are useful as backups. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Add note on restoring the docker databaseStephen Finucane2018-04-09
| | | | | | | If you back something up, you'd probably want to restore it soon enough too. Signed-off-by: Stephen Finucane <stephen@that.guru>
* trivial: Remove additional Django < 1.8 codeStephen Finucane2018-04-09
| | | | | | Yet more stuff that was missed in the previous changes. Signed-off-by: Stephen Finucane <stephen@that.guru>
* REST: Order 'filters' codeStephen Finucane2018-04-07
| | | | | | | Group custom filters and fields together followed by the actual filter sets. This makes the file a little easier to comprehend. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Add note on backing up the docker databaseStephen Finucane2018-04-07
| | | | | | I'm sick of waiting for 'parsearchive' to finish. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Add information on REST API versioningStephen Finucane2018-04-07
| | | | | | | | This isn't too prescriptive, given that so far we've only dealt with adding new fields. However, it should serve as a guide to alert devs that this stuff exists and should be a concern. Signed-off-by: Stephen Finucane <stephen@that.guru>
* REST: Use versioning for modified responsesStephen Finucane2018-04-07
| | | | | | | | | | | | | | This ensures clients are getting a consistent response if they request the old version of the API. We do this by way of extensions to the 'HyperlinkedModelSerializer' class rather than duplicating the serializers as it results in far less duplication. This approach won't work for a MAJOR version bump but, all going well, it will be a while before we have to deal with one of these. The only two fields added since API 1.0 was released, 'cover.mbox' and 'project.subject_match', are handled accordingly. Signed-off-by: Stephen Finucane <stephen@that.guru>
* tests: Split 'test_rest_api'Stephen Finucane2018-04-07
| | | | | | Lay the tests out per the main code. Signed-off-by: Stephen Finucane <stephen@that.guru>
* Use parsed subject for mboxesStephen Finucane2018-04-07
| | | | | | | | | With a recent change, we started using the original subject header instead of the one we had already cleaned up at the parsing stage. Revert this aspect of that change. Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 01b9cbb9 ("Include all email headers in mboxes")
* Include all email headers in mboxesVeronika Kabatova2018-04-07
| | | | | | | | | Solves issue #165 (Exported mboxes should include In-Reply-To, References, etc headers). Instead of including only a few chosen ones, all received headers are added to mboxes. Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* Fix incorrect autodelegation documentationVeronika Kabatova2018-04-05
| | | | | | | | | | | | The docs suggested to account for git prefixes (a/, b/) using eg. ?/patchwork/views/*. My rules didn't work so I tried bare path (patchwork/views/*) instead. Looking at the code, the prefix really is striped away (filename = '/'.join(filename.split('/')[1:])). Fix the documentation to reflect on what is really happening. Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> [dja: see 7bb0ebd78ff7 ("parser: Add patch_get_filenames()")] Signed-off-by: Daniel Axtens <dja@axtens.net>
* docker: set timezone to Australia/CanberraDaniel Axtens2018-04-05
| | | | | | | | | | | The tzinfo package isn't installed in docker, which makes the default timezone UTC. This is unfortunate: the Django TZ in settings/base.py is Australia/Canberra, and having a non-UTC TZ is good for exposing faulty assumptions about what is and isn't UTC. Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
* docs: Update reference to kernel documentationAli Alnubani2018-04-05
| | | | | | | The referenced url was moved to Documentation/process. Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
* docs: Fix package nameAli Alnubani2018-04-05
| | | | | | | | Fixed a typo that instructed to install tox instead of reno. Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
* api: Only provide JSON version of events listDaniel Axtens2018-04-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | Something is very, very slow in the d-r-f browsable API events renderer. In my MySQL test (~33k patches), the CPU time to render the events list is ~11s, and the time taken by SQL queries is only ~3s. If the JSON renderer is used, that drops to 0.2s for the entire page (because less CPU is used, and - for some as yet unknown reason - a *very* expensive db query is dropped.) In my PostgreSQL test (~100k patches), the results are even more stark: 30s of CPU time and 0.2s of DB time goes to 0.25s for the entire page. Something is seriously, seriously wrong with whatever d-r-f is doing. So, simply render the event list as unlinked JSON for now. There are a few followups we should do, but this is an important start - no-one should be able to DoS a patchwork server by just enumerating the events! In particular, we should find out: - why postgres and mysql behaviour is so different. - what on earth d-r-f is doing that makes rendering the pretty-printed version so incredibly slow. Signed-off-by: Daniel Axtens <dja@axtens.net>
* api: EventList: change select_related() to prefetch_related()Daniel Axtens2018-04-05
| | | | | | | | | | | | | | | | | | | | | select_related() creates a single giant query that JOINs the required tables together in the DB. prefetch_related() does a similar thing, but at the Django layer - for all referenced models, it makes a separate query to the DB to fetch them. This massively, massively simplifies the job the DB has to do: instead of creating a massive, sparse results table with many columns, we do 1 query for the events, and then query for only patches/cover letters/series/projects etc referenced in those 30 events. Tested with cURL + JSON renderer + Postgres w/ ~100k patches, request time went from 1.5s to 0.25s, a 6x speedup. Tested with cURL + JSON renderer + MySQL w/ ~33k patches, request time went from ~2.2s to ~0.20s, an ~11x speedup. Signed-off-by: Daniel Axtens <dja@axtens.net>
* migrations: Add missing migrationStephen Finucane2018-03-25
| | | | | | | Add a migration that was missed in an earlier change. Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 0f25d8a15 ("Add validation for regular expressions")
* Fix slow Patch counting queryDaniel Axtens2018-03-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stephen Rothwell noticed (way back in September - sorry Stephen!) that the following query is really slow on OzLabs: SELECT COUNT(*) AS "__count" FROM "patchwork_patch" INNER JOIN "patchwork_submission" ON ("patchwork_patch"."submission_ptr_id" = "patchwork_submission"."id") WHERE ("patchwork_submission"."project_id" = 14 AND "patchwork_patch"."state_id" IN (SELECT U0."id" AS Col1 FROM "patchwork_state" U0 WHERE U0."action_required" = true ORDER BY U0."ordering" ASC)); I think this is really slow because we have to join the patch and submission table to get the project id, which we need to filter the patches. Duplicate the project id in the patch table itself, which allows us to avoid the JOIN. The new query reads as: SELECT COUNT(*) AS "__count" FROM "patchwork_patch" WHERE ("patchwork_patch"."patch_project_id" = 1 AND "patchwork_patch"."state_id" IN (SELECT U0."id" AS Col1 FROM "patchwork_state" U0 WHERE U0."action_required" = true ORDER BY U0."ordering" ASC)); Very simple testing on a small, artifical Postgres instance (3 projects, 102711 patches), shows speed gains of ~1.5-5x for this query. Looking at Postgres' cost estimates (EXPLAIN) of the first query vs the second query, we see a ~1.75x improvement there too. I suspect the gains will be bigger on OzLabs. Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Daniel Axtens <dja@axtens.net>
* Add validation for regular expressionsVeronika Kabatova2018-03-21
| | | | | | | Make sure entered regexes compile before saving them. Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
* Avoid timezone confusionVeronika Kabatova2018-03-08
| | | | | | | | | | | | | | | | | | | | | | | Patchwork saves patches, comments etc with UTC timezone and reports this time when opening the patch details. However, internally generated processes such as events are reported with the instance's local time. There's nothing wrong with that and making PW timezone-aware would add useless complexity, but in a world-wide collaboration a lot of confusion may arise as the timezone is not reported at all. Instance's local time might be very different from the local time of CI integrating with PW, which is different from the local time of person dealing with it etc. Use UTC everywhere by default instead of UTC for sumbissions and local timezone for internally generated events (which is not reported). Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> [dja: - squash 2 patches: https://patchwork.ozlabs.org/patch/876744/ https://patchwork.ozlabs.org/patch/877815/ - minor changes to both patches - rejig order of migrations and adjust wording: "happened sooner" -> "happened earlier"] Tested-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
* parser: don't fail on multiple SeriesReferencesDaniel Axtens2018-03-07
| | | | | | | | | | | | | | | | | | | | | Parallel parsing would occasonally fail with: patchwork.models.MultipleObjectsReturned: get() returned more than one SeriesReference -- it returned 2! I think these are happening if you have different processes parsing e.g. 1/3 and 2/3 simultaneously: both will have a reference to 1/3, in the case of 1 it will be the msgid, in the case of 2 it will be in References. So when we come to parse 3/3, .get() finds 2 and throws the exception. This does not fix the creation of multiple series references; it just causes them to be ignored. We still have serious race conditions with series creation, but I don't yet have clear answers for them. With this patch, they will at least not stop patches from being processed - they'll just lead to wonky series, which we already have. Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
* parser: use Patch.objects.create instead of save()Daniel Axtens2018-03-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Attempts to do parallel parsing with MySQL threw the following errors: _mysql_exceptions.OperationalError: (1213, 'Deadlock found when trying to get lock; try restarting transaction') Looking at the code, it was thrown when we created a patch like this: patch = Patch(...) patch.save() The SQL statements that were being generated were weird: UPDATE "patchwork_patch" SET ... INSERT INTO "patchwork_patch" (...) VALUES (...) As far as I can tell, the update could never work, because it was trying to update a patch that didn't exist yet. My hypothesis is that Django somehow didn't quite 'get' that because of the backend complexity of the Patch model, so it tried to do an update, failed, and then tried an insert. Change the code to use Patch.objects.create, which makes the UPDATEs and the weird MySQL errors go away. Also move it up a bit earlier in the process so that if things go wrong later at least we've committed the patch to the db. Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* parser: avoid an unnecessary UPDATE of PersonDaniel Axtens2018-03-07
| | | | | | | | | | | | | | | | Analysis of SQL statements showed that when parsing an email, the row for the Person who sent the email was always getting updated. This is because the test for updating it only checks if the incoming mail has *a* name attached to the email address, and not if it has a new name. Django is not smart enough to figure that out, and so unconditionally UPDATEs the model when asked to save. Give it a hand - only update the model and save it if the new name is in fact different. Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* parser: close a TOCTTOU bug on Person creationDaniel Axtens2018-03-07
| | | | | | | | | | | | | | | | | | | | | | find_author looks up a person by email, and if they do not exist, creates a Person model, which may be saved later if the message contains something valuable. Multiple simultaneous processes can race here: both can do the SELECT, find there is no Person, and create the model. One will succeed in saving, the other will get an IntegrityError. Reduce the window by making find_author into get_or_create_author, and plumb that through. (Remove a test that specifically required find_author to *not* create). More importantly, cover the case where we lose the race, by using get_or_create which handles the race case, catching the IntegrityError internally and fetching the winning Person model. Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> [dja: post review cleanup of now-unused import] Signed-off-by: Daniel Axtens <dja@axtens.net>
* parser: Handle even more exotically broken headersDaniel Axtens2018-03-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | An archive of the Ubuntu kernel team mailing list contains a fascinating email that causes the following parse error: email.errors.HeaderParseError: header value appears to contain an embedded header: '4Mf^tnii7k\\_EnR5aobBm6Di[DZ9@AX1wJ"okBdX-UoJ>:SRn]c6DDU"qUIwfs98vF>... The broken bit seem related to a UTF-8 quoted-printable encoded section and to be from an internal attempt to break it over multiple lines: here's a snippet from the error message: '\n\t=?utf-8?q?Tnf?=\n' but interesting the header itself does not contain the new lines, so clearly something quite weird is happening behind the scenes! This only throws on header.encode(): it actually makes it through sanitise_header and into find_headers before throwing the assertion. So, try to encode in sanitize_header as a final step. Also, fix a hilarious* python bug that this exposes: whitespace-only headers cause an index error! Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* Add test for list filtering featureVeronika Kabatova2018-02-27
| | | | | Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* Implement list filteringVeronika Kabatova2018-02-27
| | | | | | | | | | | | | | | Sometimes, multiple projects reside at the same mailing list. So far, Patchwork only allowed a single project per mailing list, which made it impossible for these projects to use Patchwork (unless they did some dirty hacks). Add a new property `subject_match` to projects and implement filtering on (list_id, subject_match) match instead of solely list_id. Instance admin can specify a regex on a per-project basis when the project is created. Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
* tools: drop vagrantDaniel Axtens2018-02-27
| | | | | | | | | It served us well, but it's now outdated (Trusty, Python 3.4, etc) There is no indication that anyone uses it or keeps it up to date. Signed-off-by: Daniel Axtens <dja@axtens.net> Acked-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
* docs: X-Patchwork-Ignore doesn't work, X-Patchwork-Hint: ignore doesDaniel Axtens2018-02-24
| | | | | | Make this match what is tested for in parser.py Signed-off-by: Daniel Axtens <dja@axtens.net>
* Don't aim to have patchwork send emailDaniel Axtens2018-02-22
| | | | | | | | | | | | Getting things like SPF and DKIM right for this would be nigh-on impossible, plus it would mean sysadmins would have to let patchwork send email, which is a risk to the reputation of their systems. Let's just aim for being a read-only representation of the mailing list for now. Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
* requirements: Use 'psycopg2-binary' packageStephen Finucane2018-02-13
| | | | | | | | | | | This resolves a deprecation warning that's recently been raised: UserWarning: The psycopg2 wheel package will be renamed from release 2.8; in order to keep installing from binary please use "pip install psycopg2-binary" instead. For details see: <http://initd.org/psycopg/docs/install.html#binary-install-from-pypi>. Signed-off-by: Stephen Finucane <stephen@that.guru>
* migrations: Add missing Series.Meta, Patch.Meta changesStephen Finucane2018-02-13
| | | | | | | | These were missed in previous patches. Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 8585ea5af ("models: Use 'base_manager_name'") Fixes: ed7328fdb ("models: Series plural name is Series")
* Fix CRLF newlines upon submission changesVeronika Kabatova2018-02-09
| | | | | | | | | | | | | | | After changing submission via admin interface, CRLF newlines are suddenly present in the body. Replace them back to '\n'. The issue was found after modifying submission via admin interface using Python 2 and downloading the respective mbox file (git choked on downloaded patch because of malformed line endings). Python 3's mail module uses '\n' internally so the problem doesn't manifest there, but the content received by Django/JS is still saved in the database with CRLF line endings which shouldn't be there. Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* travis: Run pep8 for py27 onlyStephen Finucane2018-02-03
| | | | | | | | Keep run times to a minimum. Signed-off-by: Stephen Finucane <stephen@that.guru> Acked-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
generated by cgit v1.2.3 (git 2.45.0) at 2024-11-25 05:43:31 +0000