aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* Add support for 'django-dbbackup'Stephen Finucane2018-10-10
| | | | | | | | | | | 'parsemail' and 'parsearchive' are slow. When messing with models and migrations, it can be very useful to backup the database in its current state and restore it if/when you mess up. Currently, we've documented how to do this via some commands run in the shell of the container, but we can do things easier via an application designed for this purpose: 'django-dbshell'. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Add 'unreleased' to release notes index pageStephen Finucane2018-10-06
| | | | | | It's already on the main index page. Signed-off-by: Stephen Finucane <stephen@that.guru>
* Update sphinx_rtd_theme from 0.3.0 to 0.4.2pyup-bot2018-10-06
|
* views: Populate bundles for 'todo' viewStephen Finucane2018-10-06
| | | | | Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: #213
* docs: Call out Python 3 dependency for Django 2.0Stephen Finucane2018-10-06
| | | | | | Just for those that don't read the Django release notes. Signed-off-by: Stephen Finucane <stephen@that.guru>
* Add support for Django 2.1Stephen Finucane2018-10-06
| | | | | | For once, this just works. Yay! Signed-off-by: Stephen Finucane <stephen@that.guru>
* Add support for django-filter 2.0Stephen Finucane2018-10-06
| | | | | | | | This is necessary for Django 2.1 support. We retain support for django-filter 1.0 and 1.1 as 2.0 is Python 3-only. Thankfully there is essentially zero cost in doing so for now. Signed-off-by: Stephen Finucane <stephen@that.guru>
* requirements: Bump version of django-debug-toolbar to 1.10.1Stephen Finucane2018-10-06
| | | | | | Use the latest and greatest. Signed-off-by: Stephen Finucane <stephen@that.guru>
* templates: Avoid recursive callStephen Finucane2018-10-01
| | | | | | | | | | | | | | 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")
* templates: Remove trailing newlinesStephen Finucane2018-09-29
| | | | | | | These got added accidentally during merge. Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 45fa5c5c ("templates: Move additional email subjects to templates")
* views: Add error handling for user registrationStephen Finucane2018-09-29
| | | | | | | This was already present for registration confirmation but missing for initial registration. Resolve this. Signed-off-by: Stephen Finucane <stephen@that.guru>
* templates: Remove 'email_sent' attributeStephen Finucane2018-09-29
| | | | | | Further normalization to ensure all related code paths use similar code. Signed-off-by: Stephen Finucane <stephen@that.guru>
* templates: Move additional email subjects to templatesStephen Finucane2018-09-29
| | | | | | Use a uniform pattern for this stuff. Signed-off-by: Stephen Finucane <stephen@that.guru>
* templates: Keep only whole templates in the top-levelStephen Finucane2018-09-29
| | | | | | | Again, this should make this a little more understandable as it ensures a rough mapping exists between views and template names. Signed-off-by: Stephen Finucane <stephen@that.guru>
* templates: Rename additional templatesStephen Finucane2018-09-29
| | | | | | Make ALL the things consistent. Signed-off-by: Stephen Finucane <stephen@that.guru>
* templates: Move mails to separate directoryStephen Finucane2018-09-29
| | | | | | | | This makes things a little easier to parse. A couple of templates are renamed and the 'register.mail' template, which appears to be unused since commit f1e089f7, is removed. Signed-off-by: Stephen Finucane <stephen@that.guru>
* settings: Don't configure logging for parsearchiveStephen Finucane2018-09-24
| | | | | | | | | | | | | | | 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>
* requirements: Start using fixed versionsStephen Finucane2018-09-22
| | | | | | | | | | | | | | Given that 'tox' doesn't actually read any of these, there's no reason to use ranges of requirements. Instead, use the latest and greatest for live instances and rely on tox to validate behavior with older versions. The selenium dependency, which is no longer required since commit bab2895f, is removed. The psycopg2 dependency is updated to use psycopg2-binary, as this avoids the need for the libpg library and removes a deprecation warning. Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
* parser: Handle IntegrityError for cover letters, commentsStephen Finucane2018-09-20
| | | | | | | | | | | | | | | | 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. Signed-off-by: Stephen Finucane <stephen@that.guru>
* parsearchive: Fix loggingStephen Finucane2018-09-20
| | | | | | | | | | | We should use a counter normally, avoid using the counter and emit logs when more detailed output is requested, and emit nothing when no output is requested. In addition, the default logging level for the parser module is set to 'WARNING' to make it less chatty. Signed-off-by: Stephen Finucane <stephen@that.guru>
* admin: Configure 'list_select_related', 'get_queryset'Stephen Finucane2018-09-19
| | | | | | | | | | | | | | This has a significant improvement for the patch and series views. /patchwork/patch FROM: ~114 queries TO: ~14 queries /patchwork/series FROM: ~210 queries TO: ~10 queries Signed-off-by: Stephen Finucane <stephen@that.guru>
* REST: Only list checks for the given patchStephen Finucane2018-09-19
| | | | | | | | | This is either a regression or it never worked. In any case, fix it and add a test to ensure it doesn't happen again. Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Veronika Kabatova <vkabatov@redhat.com> Closes: #203
* Revert "gitignore: Ignore JSON files"Stephen Finucane2018-09-19
| | | | | | | | | This reverts commit 76750e9789ad10d45134c1ab59efa586007be984. The 'dumpdata' command is run infrequently enough, and JSON files are that common, that we shouldn't really ignore these. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Note new requirement to include a SPDX lineStephen Finucane2018-09-19
| | | | | | | | | | Add some wording around the requirement to include this line instead of the license header. Also note the requirement that all code be licensed using the 'GPL-2.0-or-later' license and add a CONTRIBUTING document, which GitHub likes. Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Veronika Kabatova <vkabatov@redhat.com>
* Update license headerStephen Finucane2018-09-19
| | | | | | | | | | | | | The FSF has a new address since 2005 that hasn't been noted in any file except the COPYING file. Rather than fix these, simply remove the headers in favour of a SPDX license header. IANAL but the combination of the header and the COPYING file in source should resolve this issue. Note that copyright notices are retained. Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: #210 Reviewed-by: Veronika Kabatova <vkabatov@redhat.com>
* Remove '__future__.absolute_import' importsStephen Finucane2018-09-19
| | | | | | | These were added as part of the Python 3 support series but are not required and can be safely removed. Signed-off-by: Stephen Finucane <stephen@that.guru>
* tox: Specify doctree directoryStephen Finucane2018-09-18
| | | | | | | | | Sphinx 1.8 has a change in where it places 'doctree' directories. Rather than ignore this directory via gitignore, specify where this directory should go. This will ensure future changes in Sphinx's behavior won't affect us. Signed-off-by: Stephen Finucane <stephen@that.guru>
* urls: Remove unused importStephen Finucane2018-09-11
| | | | | Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 5976cce6 ("urls: Remove references to Django < 1.11")
* admin: Remove unused SeriesInlineStephen Finucane2018-09-11
| | | | | | This was added in commit d67d859f but was never used. Remove it. Signed-off-by: Stephen Finucane <stephen@that.guru>
* templatetags: Remove dead tagStephen Finucane2018-09-11
| | | | | | Nothing appears to be using this so it can be removed. Signed-off-by: Stephen Finucane <stephen@that.guru>
* urls: Remove references to Django < 1.11Stephen Finucane2018-09-11
| | | | | | This was missed in the recent "add support for Django 2.0" series. Signed-off-by: Stephen Finucane <stephen@that.guru>
* tests: Remove useless todoStephen Finucane2018-09-10
| | | | | | | I'm not sure why I included this in day one: we _want_ to use the context manager like this. Signed-off-by: Stephen Finucane <stephen@that.guru>
* docs: Add release note for Django 1.8 to 1.10 removalStephen Finucane2018-09-10
| | | | | Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: e97bd5ca0 ("Remove support for Django 1.8, 1.9, 1.10")
* docs: Add release note for recent DB optimizationsStephen Finucane2018-09-10
| | | | Signed-off-by: Stephen Finucane <stephen@that.guru>
* models: Remove 'latest_series'Stephen Finucane2018-09-10
| | | | | | | | This is only used in a single view (where it probably shouldn't be used) and some tests. It's an anti-pattern that makes it too easy to shoot yourself in the foot. Remove it. Signed-off-by: Stephen Finucane <stephen@that.guru>
* Fetch maintainer information in one queryStewart Smith2018-09-10
| | | | | | | | | | | | | | | Viewing the /project/ page lists maintainers. Prior to this patch, this was done in one query to fetch the maintainer IDs, and then one query per mainatiner to get the name/email address. Now, with this patch, it's all in one query (yay joins) and saves a few ms of database queries for displaying the page. Realistically, this doesn't save us too much time as counting how many patches are there takes 99% of the database time for this page. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* Be sensible computing project patch countsStewart Smith2018-09-10
| | | | | | | | | | | | | | | | | Django actively fights constructing a query that isn't insane. So, let's go and just execute a raw one. This is all very standard SQL so should execute everywhere without a problem. With the dataset of patchwork.ozlabs.org, looking at the /project/ page for qemu-devel would take 13 queries and 1500ms, with this patch it's down to 11 queries in ~250ms. For the dataset of the netdev list, it's down to 440ms from 1500ms. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> [stephenfin: Handle projects that have no archived patches and those with all patches archived] Signed-off-by: Stephen Finucane <stephen@that.guru>
* Optimise fetching checks when displaying a patchStewart Smith2018-09-10
| | | | | | | | | | | | | Prior to this patch, a typical /patch// query for linuxppc-dev (which has about half a dozen checks per patch) took around 20 queries and 16.5ms in the database. About half of those queries were fetching the checks and who did the check. We can just do one query to get all that needed information, so we do that. This brings a page load down to 10 queries in 12ms. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* Add covering index to patchwork_submissions for /list/ queriesStewart Smith2018-09-10
| | | | | | | | | | | This gets PostgreSQL to generate *much* better query plans, gaining us about two orders of magnitude in performance on the /list/ query for the worst state project on the patchwork.ozlabs.org instance (qemu-devel). Signed-off-by: Stewart Smith <stewart@linux.ibm.com> [stephenfin: Regenerate migrations per addition of 0027 in earlier patch] Signed-off-by: Stephen Finucane <stephen@that.guru>
* Be particular over check_set and series prefetch for /list/Stewart Smith2018-09-10
| | | | | | | | At this point it shaves at most 1-2ms off the query time for /linuxppc-dev/list/ Signed-off-by: Stewart Smith <stewart@linux.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* check distinct(user) based on just user_idStewart Smith2018-09-10
| | | | | | | | | | | | | | | The logic to display the Check(s) on the patch list wants to really do a DISTINCT(user_id,context) ORDER BY DATE query, but with Django that is currently a bit Too Hard (at least for me). But what we can do is from python just use the user_id rather than the user object itself. Same functionality, no join or prefetching users. This saves a couple of hundred queries on the linuxppc/list/ view and makes loading it about 4x faster in terms of time spent in the db. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* Add covering index for /list/ queryStewart Smith2018-09-10
| | | | | | | | | | | | | | | | | | | | | | | In constructing the list of patches for a project, there are two main queries that are executed: 1) get a count() of how many patches there are 2) Get the page of results being displayed In a test dataset of ~11500 LKML patches and ~4000 others, the existing code would take around 585ms and 858ms with a cold cache and 28ms and 198ms for a warm cache. By adding a covering index, we get down to 4ms and 255ms for a cold cache, and 4ms and 143ms for a warm cache! Additionally, when there's a lot of archived or accepted patches (I used ~11000 archived out of the 15000 total in my test set) the query time goes from 28ms and 72ms down to 2ms and 33-40ms! Signed-off-by: Stewart Smith <stewart@linux.ibm.com> [stephenfin: Regenerate migrations per addition of 0027 in earlier patch] Signed-off-by: Stephen Finucane <stephen@that.guru>
* Fetch all series for patch/cover viewingStewart Smith2018-09-10
| | | | | | | | | | | | | | | | | | | e.g. a 10 comment patch goes from 26 queries in 17-20ms down to 20 queries in 12ms. A 67 comment cover letter goes from 14 queries in 16ms down to 8 queries in 8ms. So, effectively, a near 2x perf improvement. Previously, at several points we were asking for the latest series and then asking for all the series. Since there just usually aren't *that* many series, fetch them all and take the first one if we need to. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> [stephenfin: Fix typos in the template and ensure patches from all series are shown] Signed-off-by: Stephen Finucane <stephen@that.guru>
* Add index for patchwork_comment (submission_id,date)Stewart Smith2018-09-06
| | | | | | | | | | | | | | | | | | | This (at least theoretically) should speed up displaying comments on patches/cover letters. It's an index that will return rows in-order for the query that we always do ("give me the comments on this submission in date order"), rather than having to have the database server do a sort for us. I haven't been able to benchmark something locally that shows this is an actual improvement, but I don't have as large data set as various production instances. The query plan does look a bit nicer though. Although the benefit of index maintenance versus how long it takes to sort things is a good question. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> [stephenfin: Regenerate migrations per addition of 0027 in earlier patch] Signed-off-by: Stephen Finucane <stephen@that.guru>
* 4x performance improvement for viewing patch with many commentsStewart Smith2018-08-31
| | | | | | | | | | | | | | | | | | | | | | Using the example of id:20180720035941.6844-1-khandual@linux.vnet.ibm.com with my test dataset of a chunk of a variety of mailing lists, has this cover letter have 67 comments from a variety of people. Thus, it's on the larger side of things. Originally, displaying the /patch/550/ for this (redirected to /cover) would take 81 SQL queries in ~60ms on my laptop. After this optimisation, it's down to 14 queries in 14ms. When the cache is cold, it's down to 32ms from 83ms. The effect of this patch is to execute a join in the database to get the submitter information for each comment at the same time as getting all the comments rather than doing a one-by-one lookup after the fact. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
* Improve patch listing performance (~3x)Stewart Smith2018-08-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There's two main bits that are really expensive when composing the list of patches for a project: the query getting the list, and the query finding the series for each patch. If we look at the query getting the list, it gets a lot of unnecessary fields such as 'headers' and 'content', even though we tell Django not to. It turns out that Django seems to ignore the Submission relationship and I have no idea how to force it to ignore that thing (defer doesn't work) but if we go only, then it works okay. From my import of ~8000 messages for a few projects, my laptop query time (MySQL, as setup by whatever the docker-compose things do) goes from: http://localhost:8000/project/linux-kernel/list/ FROM: 342ms SQL queries cold cache, 268ms warm cache TO: 118ms SQL queries cold cache, 88ms warm cache Which is... non trivial to say the least. The big jump is the patches.only change, and the removal of ordering on the patchseries takes a further 10ms off. For some strange reason, it seems rather hard to tell Django that you don't care what order the results come back in for that query (if we do, then the db server has to do a sort rather than just return each row) Signed-off-by: Stewart Smith <stewart@linux.ibm.com> [stephenfin: Add missing migration that had been squashed into a later migration] Signed-off-by: Stephen Finucane <stephen@that.guru>
* docker: Use heredocs where possibleStephen Finucane2018-08-30
| | | | | | | | | | This was suggested in a recent review [1]. Make it happen. [1] http://patchwork.ozlabs.org/patch/933979/#1941584 Signed-off-by: Stephen Finucane <stephen@that.guru> Suggested-by: Petr Vorel <petr.vorel@gmail.com> Reviewed-by: Petr Vorel <pvorel@suse.cz>
* docs: Fix documentation of REST_RESULTS_PER_PAGE settingAndrew Donnellan2018-08-26
| | | | | | | | | | In 8fe11180a1a5 ("REST: Add new setting for maximum API page size") I accidentally deleted the versionadded information for REST_RESULTS_PER_PAGE. Restore it. Fixes: 8fe11180a1a5 ("REST: Add new setting for maximum API page size") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
* pwclient: Fix pwclient am output formattingPetr Vorel2018-08-26
| | | | | | | | | | | | | | | | 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>
* REST: Add new setting for maximum API page sizeAndrew Donnellan2018-08-26
| | | | | | | | | | | | | | | | | | 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>