Add request-level caching for privileged queries.

Add request-level caching for privileged queries.

BUG=catapult:#2807

Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/4f3a13e5fdea583299ecf64e530a4b42201a7fcb

[sullivan, 2016-09-19 14:47:23]

The CQ bit was checked by sullivan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 14:47:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 14:51:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-19 14:51:43]

Dry run: Try jobs failed on following builders:
  Catapult Mac Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Ma...)

[sullivan, 2016-09-19 15:04:52]

The CQ bit was checked by sullivan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 15:05:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 15:08:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-19 15:08:53]

Dry run: Try jobs failed on following builders:
  Catapult Mac Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Ma...)

[sullivan, 2016-09-19 15:18:35]

The CQ bit was checked by sullivan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 15:18:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 15:22:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-19 15:22:34]

Dry run: Try jobs failed on following builders:
  Catapult Mac Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Ma...)

[sullivan, 2016-09-19 21:40:10]

The CQ bit was checked by sullivan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 21:40:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sullivan, 2016-09-19 21:40:26]

sullivan@chromium.org changed reviewers:
+ aiolos@chromium.org, benjhayden@chromium.org, dtu@chromium.org,
eakuefner@chromium.org

[sullivan, 2016-09-19 21:40:27]

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/dat...
File dashboard/dashboard/datastore_hooks.py (right):

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/dat...
dashboard/dashboard/datastore_hooks.py:107: return True
Switched this from False->True because it turns out a lot more unit tests hit
this code path when I move it up a level, and the assumption has been that it's
True in those tests.

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/lay...
File dashboard/dashboard/layered_cache_test.py (right):

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/lay...
dashboard/dashboard/layered_cache_test.py:32: self.PatchDatastoreHooksRequest()
These are needed because the unit tests now hit the code that checks
webapp2.get_request().registry.

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/tes...
File dashboard/dashboard/testing_common.py (right):

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/tes...
dashboard/dashboard/testing_common.py:99: pass
This is now needed because a lot of unit tests call SetCurrentUser to change
internal_only in unit tests. In real requests, request.registry gets reset in
separate requests, but if a single unit test makes multiple requests, the ones
in the same unit test keep the cached request.registry. So reset it when
resetting the user.

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/upd...
File dashboard/dashboard/update_test_suites_test.py (right):

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/upd...
dashboard/dashboard/update_test_suites_test.py:72:
request.registry['privileged_cached'] = False
This seems to have been the only unit test affected by switching the default
from False->True if there is no request object (the direct calls to
update_test_suites.FetchCachedTestSuites() have different behavior now). It was
expecting *not* to be privileged, so set that.

[commit-bot: I haz the power, 2016-09-19 21:41:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-19 21:41:25]

Dry run: Try jobs failed on following builders:
  Catapult Android Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20An...)
  Catapult Linux Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Li...)

[benjhayden, 2016-09-19 22:11:10]

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/dat...
File dashboard/dashboard/datastore_hooks.py (right):

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/dat...
dashboard/dashboard/datastore_hooks.py:128: # DO NOT CACHE the result of a
SINGLE privilige elevation.
privil_e_ge

[benjhayden, 2016-09-19 22:14:53]

lgtm

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/upd...
File dashboard/dashboard/update_test_suites_test.py (right):

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/upd...
dashboard/dashboard/update_test_suites_test.py:101: del
request.registry['privileged_cached']
Is this needed?

[sullivan, 2016-09-20 02:19:32]

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/dat...
File dashboard/dashboard/datastore_hooks.py (right):

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/dat...
dashboard/dashboard/datastore_hooks.py:128: # DO NOT CACHE the result of a
SINGLE privilige elevation.
On 2016/09/19 22:11:10, benjhayden wrote:
> privil_e_ge

Done.

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/upd...
File dashboard/dashboard/update_test_suites_test.py (right):

https://codereview.chromium.org/2350113002/diff/60001/dashboard/dashboard/upd...
dashboard/dashboard/update_test_suites_test.py:101: del
request.registry['privileged_cached']
On 2016/09/19 22:14:53, benjhayden wrote:
> Is this needed?

Done.

[sullivan, 2016-09-20 02:19:36]

The CQ bit was checked by sullivan@chromium.org

[sullivan, 2016-09-20 02:19:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
benjhayden@chromium.org
Link to the patchset: https://codereview.chromium.org/2350113002/#ps80001
(title: "Addressed review comments")

[commit-bot: I haz the power, 2016-09-20 02:19:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 02:20:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-20 02:20:56]

Try jobs failed on following builders:
  Catapult Android Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20An...)
  Catapult Linux Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Li...)

[sullivan, 2016-09-20 14:10:05]

The CQ bit was checked by sullivan@chromium.org

[commit-bot: I haz the power, 2016-09-20 14:10:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 14:44:33]

Description was changed from

==========
Add request-level caching for privileged queries.

BUG=catapult:#2807
==========

to

==========
Add request-level caching for privileged queries.

BUG=catapult:#2807

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-09-20 14:44:34]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

[sullivan, 2016-09-20 22:23:25]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2356993002/ by sullivan@chromium.org.

The reason for reverting is: This causes some errors for signed-out users when
viewing charts. Reverting while I investigate..