365aee704600a83465d9d272c7251dd24b1f92a6: mozreview: prevent pushing commits with multiple bugs (bug 1135941) r?gps
Dan Minor <dminor@mozilla.com> - Wed, 13 Jan 2016 13:01:17 -0500 - rev 362118
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: prevent pushing commits with multiple bugs (bug 1135941) r?gps It would be nice to automatically create multiple reviews when multiple bugs are specified, but that seems like a larger project. In the mean time, we should prevent people from shooting themselves in the foot, and disallow pushing commits that specify more than one bug.
e085a8725612aefca2a8c80e1d01c63f2348d33f: mozreview: add default reviewers the first time we add a diffset (bug 1239524); r=smacleod
Gregory Szorc <gps@mozilla.com> - Wed, 13 Jan 2016 16:47:58 -0800 - rev 362117
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: add default reviewers the first time we add a diffset (bug 1239524); r=smacleod The web UI enforces the requirement that a review request have reviewers. This behavior is different from the web API, which doesn't enforce this requirement. As part of converting to the batch submission API, we failed to carry over the setting of default reviewers from DiffResource.create() from the Review Board Web API. This commit fixes that.
f322a2a99918a830904be58cb676aa57b4038764: hgserver: fix TypeError in "edit the description" action (bug 1239272)
Gregory Szorc <gps@mozilla.com> - Wed, 13 Jan 2016 15:38:08 -0800 - rev 362116
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
hgserver: fix TypeError in "edit the description" action (bug 1239272) And added a test that description editing actually works as advertised.
c071b9f563acbc683c69ff033fc561f703b5ca5c: mozreview: define approval bg and border as less variables; r=smacleod
Mauro Doglio <mdoglio@mozilla.com> - Wed, 13 Jan 2016 19:10:45 +0000 - rev 362115
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: define approval bg and border as less variables; r=smacleod
ccb5b3af1e209d665df1b8f1ae8c9005cd046143: mozreview: show reviewer status in commit list (bug 1214614); r=smacleod
Mauro Doglio <mdoglio@mozilla.com> - Fri, 18 Dec 2015 16:32:59 +0000 - rev 362114
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: show reviewer status in commit list (bug 1214614); r=smacleod This adds a green background to the reviewer name when her last review has a ship-it. Open issues are not taken into account yet, so a `fix it, then ship it` will result in a green background on the reviewer name and a yellow icon for the whole commit status.
ae98f3b686e4db0c227fa2332afdb299aef70b9e: mozreview: add reviewers status to review request summary (bug 1214614); r=mcote,smacleod
Mauro Doglio <mdoglio@mozilla.com> - Wed, 13 Jan 2016 16:09:56 +0000 - rev 362113
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: add reviewers status to review request summary (bug 1214614); r=mcote,smacleod
88936b566874b7ca92f6a283c7d7f99bb88a3397: mozreview: use transaction context manager properly (bug 1239452); r=smacleod
Gregory Szorc <gps@mozilla.com> - Wed, 13 Jan 2016 15:04:46 -0800 - rev 362112
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: use transaction context manager properly (bug 1239452); r=smacleod Apparently I didn't test this before landing. I swear I did though :/ I also fixed an issue where we were forgetting to unchange user.username in the case where we don't change the username.
b7979b2c6de6dc090479b3d7c310489f3005a746: mozreview: use transaction.atomic to recover from IntegrityError (bug 1239452); r=smacleod
Gregory Szorc <gps@mozilla.com> - Wed, 13 Jan 2016 14:38:45 -0800 - rev 362111
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: use transaction.atomic to recover from IntegrityError (bug 1239452); r=smacleod For reasons similar to https://stackoverflow.com/questions/20130507/django-transactionmanagementerror-when-using-signals
b84ebb1cd00a7dc6994af833fd8241fbbbe8fbca: mozreview: refactor updating of usernames (bug 1239452); r=smacleod
Gregory Szorc <gps@mozilla.com> - Wed, 13 Jan 2016 14:36:28 -0800 - rev 362110
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: refactor updating of usernames (bug 1239452); r=smacleod Before, we had a single save() and an except block that assumed the exception was due to username reasons. While unlikely, other failures could occur. So we move the logic for handling username save failures into the username change block. While I was here, the logic changed slightly.
68171052f808a382d1a6b212e30c96d34dac1ca8: mozreview: add more logging around creation of users; r=smacleod
Gregory Szorc <gps@mozilla.com> - Wed, 13 Jan 2016 14:36:17 -0800 - rev 362109
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: add more logging around creation of users; r=smacleod To help with further forensic debugging.
711e22b2055b9626dde846bc73b10cd916bc9f16: rbbz: format user list properly; r=smacleod
Gregory Szorc <gps@mozilla.com> - Wed, 13 Jan 2016 14:36:06 -0800 - rev 362108
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
rbbz: format user list properly; r=smacleod Space around comma was wrong.
beab308418a9e9071f26ffbaa41d62507adf641b: reviewboard: use demandimport.deactivated; r=smacleod
Gregory Szorc <gps@mozilla.com> - Wed, 13 Jan 2016 12:12:42 -0800 - rev 362107
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
reviewboard: use demandimport.deactivated; r=smacleod This is the proper way to temporarily deactivate demandimport, as it will restore the previous state instead of assuming it was enabled to begin with.
4dfd481dc75751c34b7f8ce54ab3cd5dccdd29b9: reviewboard: don't use ReviewBoardClient as a context manager; r=smacleod
Gregory Szorc <gps@mozilla.com> - Mon, 11 Jan 2016 15:57:45 -0800 - rev 362106
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
reviewboard: don't use ReviewBoardClient as a context manager; r=smacleod Because it no longer needs to be one. The diff looks painful but it is mostly whitespace.
8e78085c5add6a463cccd0195793b3d9e65999de: reviewboard: use built-in mechanism to not save cookies; r=smacleod
Gregory Szorc <gps@mozilla.com> - Mon, 11 Jan 2016 15:57:31 -0800 - rev 362105
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
reviewboard: use built-in mechanism to not save cookies; r=smacleod The hack around using a temporary file as a cookie manager was necessary because older versions of RBTools didn't support not saving cookies. New versions do. So remove the hack and use the built-in mechanism. This function no longer needs to be a context manager. This will be removed in a subsequent commit.
b2c6575364a1d60da085d8e90595f883451b4656: reviewboard: use built-in mechanism to disable caching; r=smacleod
Gregory Szorc <gps@mozilla.com> - Mon, 11 Jan 2016 15:57:20 -0800 - rev 362104
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
reviewboard: use built-in mechanism to disable caching; r=smacleod Modern versions of RBTools allow caching to be disabled by passing an argument to the constructor. This means our hack to define a custom transport class with caching disabled can be removed. We also disable caching in similar code that was also wrapping a RBTools client.
2055f60e3d4c31169ba49b0e4e64122beb9add26: mozreview: use RBTools 0.7.5 everywhere; r=smacleod
Gregory Szorc <gps@mozilla.com> - Mon, 11 Jan 2016 15:56:38 -0800 - rev 362103
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: use RBTools 0.7.5 everywhere; r=smacleod We previously ran a mix of 0.6.1 and 0.7.2. Let's get on a consistent, modern release. We couldn't upgrade RBTools previously because of bugs around Unicode handling. These are apparently all fixed.
16bc1f7bef615ba3d5e6060848ebfbe9efb60a0b: reviewboard: move review submission logic out of pushhooks.py; r=smacleod
Gregory Szorc <gps@mozilla.com> - Mon, 11 Jan 2016 15:56:23 -0800 - rev 362102
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
reviewboard: move review submission logic out of pushhooks.py; r=smacleod This code does very little now and doesn't need to exist in pushhooks.py. Move it to proto.py. Yes, the future Git implementation will need to reinvent this wheel. But the code is mostly boilerplate: I'm not concerned at the moment.
1cc42ef4b24f5b9e25f737ceba51ddc0ef588ea1: mozreview: cache reviewer to User lookups (bug 1229468); r=smacleod
Gregory Szorc <gps@mozilla.com> - Mon, 11 Jan 2016 15:56:10 -0800 - rev 362101
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: cache reviewer to User lookups (bug 1229468); r=smacleod The previous commit regressed the total number of BMO HTTP requests because it stopped caching user lookups. This commit reintroduces that caching. The effect of this commit is the total number of HTTP requests to BMO is reduced to the level they were at before the previous commit, or a net reduction of 18 requests from the previous commit. Wall time didn't decrease significantly. But I reckon this is because of the no latency, no load environment. I have a feeling this series will have a dramatic impact on production performance.
252a96904fba6c1d1bc11caf6a40bf6d09dfe3d7: mozreview: create child review requests from batch API (bug 1229468); r=smacleod, dminor
Gregory Szorc <gps@mozilla.com> - Wed, 13 Jan 2016 12:31:01 -0800 - rev 362100
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: create child review requests from batch API (bug 1229468); r=smacleod, dminor The final step for moving the creation of review series into a unified web API is moving the management of child review requests to the new API, which this commit does. This commit is rather large. Unfortunately, there is no easy way to incrementally move this code since managing the state of child review requests is so complicated and intertwined. As part of migrating the code, the overall structure changed substantially. This is because we're able to do things differently from the context of the web api that we weren't from outside. For example, we can query the user database directly instead of going through the search interface. The best way to review this change is probably to open a copy of pushhooks.py in a separate window and read it alongside the new code in batch_review_request.py. Fortunately, our test coverage of review state is pretty robust. And, as you can see, there are no test changes as a result of this refactor! Presumably the new behavior is identical to the old behavior! There is still room to optimize this code. For example, the old code maintained a cache of reviewer lookups. This was dropped as part of the refactor. It can and should be added in later. The practical effect of this change is that review submission and publishing now requires far fewer HTTP requests to the Review Board Web API. On a local test cluster, submitting and publishing a review series of 20 commits with a single reviewer required the following: Before: 12.0s 134 BMO requests 158 RB requests After: 10.0s 154 BMO requests 54 RB requests Delta: -2.0s +20 HTTP requests -104 HTTP requests (The HTTP request count includes requests during cluster startup, so it is artificially high to start.) The regression in BMO HTTP requests is almost certainly due to redundant user lookups. This will be corrected later.
64bc74f4b91a7730c7d03c4fcbe9f4a723813a75: mozreview: copy more read-only processing code (bug 1229468); r=smacleod, dminor
Gregory Szorc <gps@mozilla.com> - Tue, 12 Jan 2016 15:07:29 -0800 - rev 362099
Push 16998 by rwood@mozilla.com at Mon, 02 May 2016 19:42:03 +0000
mozreview: copy more read-only processing code (bug 1229468); r=smacleod, dminor This is basically copying a lot of code from pushhooks.py. We don't yet make any database modification as part of this code. This just proves that a rough 1:1 copy continues to work.
(0) -300000 -100000 -30000 -10000 -3000 -1000 -300 -100 -50 -20 +20 +50 +100 +300 +1000 +3000 +10000 +30000 +100000 +300000 tip