tools/android/loading Switch the GCE worker to pull queues

tools/android/loading Switch the GCE worker to pull queues

The GCE worker now uses pull queues instead of HTTP requests.

Committed: https://crrev.com/56df5e65bb133509403661c090e18617b1910d36
Cr-Commit-Position: refs/heads/master@{#388265}

[droger, 2016-04-18 16:17:23]

Patchset #1 (id:1) has been deleted

[droger, 2016-04-18 16:17:29]

Patchset #1 (id:20001) has been deleted

[droger, 2016-04-18 16:17:36]

Patchset #1 (id:40001) has been deleted

[droger, 2016-04-18 16:17:42]

Patchset #1 (id:60001) has been deleted

[droger, 2016-04-18 16:21:00]

Patchset #1 (id:80001) has been deleted

[droger, 2016-04-18 16:21:49]

droger@chromium.org changed reviewers:
+ blundell@chromium.org, lizeb@chromium.org

[droger, 2016-04-18 16:21:50]

blundell: for review
lizeb: FYI, but feel free to review as well.

We'll discuss this CL offline as well.

[droger, 2016-04-18 16:22:31]

Description was changed from

==========
tools/android/loading Switch the GCE worker to pull queues
==========

to

==========
tools/android/loading Switch the GCE worker to pull queues

The GCE worker now uses pull queues instead of HTTP requests.
==========

[Benoit L, 2016-04-19 08:47:41]

Thanks!

A few questions, mostly for my own education.

Also, can you prefer logging over print for logging purposes? This makes it
easier to categorize logs, redirect to files, add timestamps, filter, etc.

https://codereview.chromium.org/1895033002/diff/100001/tools/android/loading/...
File tools/android/loading/gce/worker.py (right):

https://codereview.chromium.org/1895033002/diff/100001/tools/android/loading/...
tools/android/loading/gce/worker.py:76: project=project, taskqueue=queue_name,
numTasks=1, leaseSecs=60,
Is 60s enough?

https://codereview.chromium.org/1895033002/diff/100001/tools/android/loading/...
tools/android/loading/gce/worker.py:99: self._Finalize()
Does it mean that this is all or nothing? That is, if for some reason we have a
crash at some point, we lose all the progress already made? (but still keep
un-indexed files around).

https://codereview.chromium.org/1895033002/diff/100001/tools/android/loading/...
tools/android/loading/gce/worker.py:180: repeat_count =
int(params.get('repeat_count', '1'))
are params necessarily strings?

[droger, 2016-04-19 11:00:00]

https://codereview.chromium.org/1895033002/diff/100001/tools/android/loading/...
File tools/android/loading/gce/worker.py (right):

https://codereview.chromium.org/1895033002/diff/100001/tools/android/loading/...
tools/android/loading/gce/worker.py:76: project=project, taskqueue=queue_name,
numTasks=1, leaseSecs=60,
On 2016/04/19 08:47:41, Benoit L wrote:
> Is 60s enough?

You're right, it may not be enough in some cases.
Changing to 3 minutes.

https://codereview.chromium.org/1895033002/diff/100001/tools/android/loading/...
tools/android/loading/gce/worker.py:99: self._Finalize()
On 2016/04/19 08:47:41, Benoit L wrote:
> Does it mean that this is all or nothing? That is, if for some reason we have
a
> crash at some point, we lose all the progress already made? (but still keep
> un-indexed files around).

Yes for now, because the trace database does not support concurrent access yet.
Fixing this is very high on my list, but is not done in this CL.

https://codereview.chromium.org/1895033002/diff/100001/tools/android/loading/...
tools/android/loading/gce/worker.py:180: repeat_count =
int(params.get('repeat_count', '1'))
On 2016/04/19 08:47:41, Benoit L wrote:
> are params necessarily strings?

Good point, changed to int.

[blundell, 2016-04-19 11:31:22]

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
File tools/android/loading/gce/worker.py (right):

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
tools/android/loading/gce/worker.py:21: from frontend.clovis_task import
ClovisTask
I think that rather than have the backend access the frontend we should have a
common/ directory for code shared between them.

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
tools/android/loading/gce/worker.py:30: """See README.md for the config
format"""
nit: You're missing periods at the end of many of your docstrings (in the other
CL as well).

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
tools/android/loading/gce/worker.py:33: # Separate the cloud storage path into
the bucket and the base path under
Please move lines 33-36 to be directly above line 40.

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
tools/android/loading/gce/worker.py:75: response = task_api.tasks().lease(
Could we put most of the code from lines 75 through 93 into a helper function
like GetNextClovisTask()? It could return the task, returning None if it
couldn't find a task.

[droger, 2016-04-19 12:10:51]

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
File tools/android/loading/gce/worker.py (right):

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
tools/android/loading/gce/worker.py:21: from frontend.clovis_task import
ClovisTask
On 2016/04/19 11:31:22, blundell wrote:
> I think that rather than have the backend access the frontend we should have a
> common/ directory for code shared between them.

Unfortunately this is not possible. Any file needed by the frontend needs to be
under frontend/.

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
tools/android/loading/gce/worker.py:30: """See README.md for the config
format"""
On 2016/04/19 11:31:22, blundell wrote:
> nit: You're missing periods at the end of many of your docstrings (in the
other
> CL as well).

Done, in the other CL as well.

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
tools/android/loading/gce/worker.py:33: # Separate the cloud storage path into
the bucket and the base path under
On 2016/04/19 11:31:22, blundell wrote:
> Please move lines 33-36 to be directly above line 40.

Done.

https://codereview.chromium.org/1895033002/diff/120001/tools/android/loading/...
tools/android/loading/gce/worker.py:75: response = task_api.tasks().lease(
On 2016/04/19 11:31:22, blundell wrote:
> Could we put most of the code from lines 75 through 93 into a helper function
> like GetNextClovisTask()? It could return the task, returning None if it
> couldn't find a task.

Done.

[droger, 2016-04-19 13:51:16]

On 2016/04/19 08:47:41, Benoit L wrote:
> Thanks!
> 
> A few questions, mostly for my own education.
> 
> Also, can you prefer logging over print for logging purposes? This makes it
> easier to categorize logs, redirect to files, add timestamps, filter, etc.

Done using a Logger instance.

[droger, 2016-04-19 13:51:35]

Patchset #4 (id:160001) has been deleted

[blundell, 2016-04-19 13:54:14]

lgtm

[Benoit L, 2016-04-19 13:58:58]

On 2016/04/19 13:54:14, blundell wrote:
> lgtm

lgtm, thanks!

[droger, 2016-04-19 17:24:55]

The CQ bit was checked by droger@chromium.org

[droger, 2016-04-19 17:24:56]

The patchset sent to the CQ was uploaded after l-g-t-m from lizeb@chromium.org,
blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1895033002/#ps220001
(title: " ")

[commit-bot: I haz the power, 2016-04-19 17:25:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1895033002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1895033002/220001

[commit-bot: I haz the power, 2016-04-19 19:10:52]

Description was changed from

==========
tools/android/loading Switch the GCE worker to pull queues

The GCE worker now uses pull queues instead of HTTP requests.
==========

to

==========
tools/android/loading Switch the GCE worker to pull queues

The GCE worker now uses pull queues instead of HTTP requests.
==========

[commit-bot: I haz the power, 2016-04-19 19:10:53]

Committed patchset #6 (id:220001)

[commit-bot: I haz the power, 2016-04-22 19:14:38]

Description was changed from

==========
tools/android/loading Switch the GCE worker to pull queues

The GCE worker now uses pull queues instead of HTTP requests.
==========

to

==========
tools/android/loading Switch the GCE worker to pull queues

The GCE worker now uses pull queues instead of HTTP requests.

Committed: https://crrev.com/56df5e65bb133509403661c090e18617b1910d36
Cr-Commit-Position: refs/heads/master@{#388265}
==========

[commit-bot: I haz the power, 2016-04-22 19:14:39]

Patchset 6 (id:??) landed as
https://crrev.com/56df5e65bb133509403661c090e18617b1910d36
Cr-Commit-Position: refs/heads/master@{#388265}