Add swarming task for upload_dm_results

Add swarming task for upload_dm_results

BUG=skia:5719
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2360203004

Committed: https://skia.googlesource.com/skia/+/9766f0016669029fcdc49f00ceaaf66b01b03f4a

[borenet2, 2016-09-23 15:06:30]

Description was changed from

==========
Add swarming task for upload_dm_results

BUG=skia:5719
==========

to

==========
Add swarming task for upload_dm_results

BUG=skia:5719
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2360203004
==========

[borenet, 2016-09-23 15:55:36]

Description was changed from

==========
Add swarming task for upload_dm_results

BUG=skia:5719
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2360203004
==========

to

==========
Add swarming task for upload_dm_results

BUG=skia:5719
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2360203004
==========

[borenet, 2016-09-23 15:56:38]

borenet@google.com changed reviewers:
+ borenet@google.com, jcgregorio@google.com, rmistry@google.com,
stephana@google.com

[borenet, 2016-09-23 15:56:39]

Confirmed that we can upload to the old chromium-skia-gm bucket, but I'm
concerned about the permissions there. PTAL and provide any insight on some
questions I posed.

https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
File infra/bots/recipes/upload_dm_results.py (right):

https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
infra/bots/recipes/upload_dm_results.py:46: # TODO(borenet): Are the permissions
correct? We want only Google-readable.
It *seems* to be correct, eg.

Old upload:
http://chromium-skia-gm.commondatastorage.googleapis.com/dm-images-v1/0010007...

New upload:
http://chromium-skia-gm.commondatastorage.googleapis.com/dm-images-v1-test/00...

The old upload does an individual upload per file with multiple worker threads.
It uploads each file to a temporary location, in case the upload gets
interrupted, with the pre-defined "private" ACL. Then, it moves the file to its
intended location and then sets the fine-grained Google-readable ACL on the
file. AFAICT the newer gsutil still doesn't support uploading a directory at a
time with a fine-grained ACL. I guess in this case we're inheriting the bucket
defaults.  IMO the simplest way forward is to create a new bucket which has the
right fine-grained ACLs as bucket defaults, so our uploader doesn't need to deal
with that.

https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
infra/bots/recipes/upload_dm_results.py:56: # longer a concept. What should we
use instead?
For nanobench results we don't use a build number; the builder name and revision
are enough to identify the results we want, and we add a timestamp to prevent
collisions for any duplicates.  Would that work here as well?

[stephana, 2016-09-26 16:11:24]

https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
File infra/bots/recipes/upload_dm_results.py (right):

https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
infra/bots/recipes/upload_dm_results.py:46: # TODO(borenet): Are the permissions
correct? We want only Google-readable.
On 2016/09/23 15:56:39, borenet wrote:
> It *seems* to be correct, eg.
> 
> Old upload:
>
http://chromium-skia-gm.commondatastorage.googleapis.com/dm-images-v1/0010007...
> 
> New upload:
>
http://chromium-skia-gm.commondatastorage.googleapis.com/dm-images-v1-test/00...
> 
> The old upload does an individual upload per file with multiple worker
threads.
> It uploads each file to a temporary location, in case the upload gets
> interrupted, with the pre-defined "private" ACL. Then, it moves the file to
its
> intended location and then sets the fine-grained Google-readable ACL on the
> file. AFAICT the newer gsutil still doesn't support uploading a directory at a
> time with a fine-grained ACL. I guess in this case we're inheriting the bucket
> defaults.  IMO the simplest way forward is to create a new bucket which has
the
> right fine-grained ACLs as bucket defaults, so our uploader doesn't need to
deal
> with that.
We should set this to the new bucket before we land. I am testing right now that
make sure that the default permissions are correct and work for Gold. But +100
for not setting the ACLs in the scripts and set the right default permissions
instead.

https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
infra/bots/recipes/upload_dm_results.py:56: # longer a concept. What should we
use instead?
On 2016/09/23 15:56:39, borenet wrote:
> For nanobench results we don't use a build number; the builder name and
revision
> are enough to identify the results we want, and we add a timestamp to prevent
> collisions for any duplicates.  Would that work here as well?

Per our offline conversation we should inject a timestamp (ms since the epoch)
into the path, to make sure we don't overwrite runs within the same hour.
Otherwise this should be fine.

[borenet, 2016-09-26 17:35:38]

Ok, I think this is ready to go.

https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
File infra/bots/recipes/upload_dm_results.py (right):

https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
infra/bots/recipes/upload_dm_results.py:46: # TODO(borenet): Are the permissions
correct? We want only Google-readable.
On 2016/09/26 16:11:24, stephana wrote:
> On 2016/09/23 15:56:39, borenet wrote:
> > It *seems* to be correct, eg.
> > 
> > Old upload:
> >
>
http://chromium-skia-gm.commondatastorage.googleapis.com/dm-images-v1/0010007...
> > 
> > New upload:
> >
>
http://chromium-skia-gm.commondatastorage.googleapis.com/dm-images-v1-test/00...
> > 
> > The old upload does an individual upload per file with multiple worker
> threads.
> > It uploads each file to a temporary location, in case the upload gets
> > interrupted, with the pre-defined "private" ACL. Then, it moves the file to
> its
> > intended location and then sets the fine-grained Google-readable ACL on the
> > file. AFAICT the newer gsutil still doesn't support uploading a directory at
a
> > time with a fine-grained ACL. I guess in this case we're inheriting the
bucket
> > defaults.  IMO the simplest way forward is to create a new bucket which has
> the
> > right fine-grained ACLs as bucket defaults, so our uploader doesn't need to
> deal
> > with that.
> We should set this to the new bucket before we land. I am testing right now
that
> make sure that the default permissions are correct and work for Gold. But +100
> for not setting the ACLs in the scripts and set the right default permissions
> instead. 

Changed to use the new bucket, and the default permissions there look correct to
me.

https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
infra/bots/recipes/upload_dm_results.py:56: # longer a concept. What should we
use instead?
On 2016/09/26 16:11:24, stephana wrote:
> On 2016/09/23 15:56:39, borenet wrote:
> > For nanobench results we don't use a build number; the builder name and
> revision
> > are enough to identify the results we want, and we add a timestamp to
prevent
> > collisions for any duplicates.  Would that work here as well?
> 
> Per our offline conversation we should inject a timestamp (ms since the epoch)
> into the path, to make sure we don't overwrite runs within the same hour.
> Otherwise this should be fine. 

Added the timestamp.

[borenet, 2016-09-26 17:40:01]

The CQ bit was checked by borenet@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 17:40:08]

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-26 17:49:58]

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

[commit-bot: I haz the power, 2016-09-26 17:49:59]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-ShuttleA-GPU-GTX660-x86_64-Release-GN-Trybot on
master.client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU...)

[borenet, 2016-09-26 18:40:01]

The CQ bit was checked by borenet@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 18:40: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-26 19:25:11]

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

[commit-bot: I haz the power, 2016-09-26 19:25:12]

Dry run: This issue passed the CQ dry run.

[stephana, 2016-09-26 19:37:15]

On 2016/09/26 17:35:38, borenet wrote:
> Ok, I think this is ready to go.
> 
>
https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
> File infra/bots/recipes/upload_dm_results.py (right):
> 
>
https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
> infra/bots/recipes/upload_dm_results.py:46: # TODO(borenet): Are the
permissions
> correct? We want only Google-readable.
> On 2016/09/26 16:11:24, stephana wrote:
> > On 2016/09/23 15:56:39, borenet wrote:
> > > It *seems* to be correct, eg.
> > > 
> > > Old upload:
> > >
> >
>
http://chromium-skia-gm.commondatastorage.googleapis.com/dm-images-v1/0010007...
> > > 
> > > New upload:
> > >
> >
>
http://chromium-skia-gm.commondatastorage.googleapis.com/dm-images-v1-test/00...
> > > 
> > > The old upload does an individual upload per file with multiple worker
> > threads.
> > > It uploads each file to a temporary location, in case the upload gets
> > > interrupted, with the pre-defined "private" ACL. Then, it moves the file
to
> > its
> > > intended location and then sets the fine-grained Google-readable ACL on
the
> > > file. AFAICT the newer gsutil still doesn't support uploading a directory
at
> a
> > > time with a fine-grained ACL. I guess in this case we're inheriting the
> bucket
> > > defaults.  IMO the simplest way forward is to create a new bucket which
has
> > the
> > > right fine-grained ACLs as bucket defaults, so our uploader doesn't need
to
> > deal
> > > with that.
> > We should set this to the new bucket before we land. I am testing right now
> that
> > make sure that the default permissions are correct and work for Gold. But
+100
> > for not setting the ACLs in the scripts and set the right default
permissions
> > instead. 
> 
> Changed to use the new bucket, and the default permissions there look correct
to
> me.
> 
>
https://codereview.chromium.org/2360203004/diff/60001/infra/bots/recipes/uplo...
> infra/bots/recipes/upload_dm_results.py:56: # longer a concept. What should we
> use instead?
> On 2016/09/26 16:11:24, stephana wrote:
> > On 2016/09/23 15:56:39, borenet wrote:
> > > For nanobench results we don't use a build number; the builder name and
> > revision
> > > are enough to identify the results we want, and we add a timestamp to
> prevent
> > > collisions for any duplicates.  Would that work here as well?
> > 
> > Per our offline conversation we should inject a timestamp (ms since the
epoch)
> > into the path, to make sure we don't overwrite runs within the same hour.
> > Otherwise this should be fine. 
> 
> Added the timestamp.
lgtm

[borenet, 2016-09-27 13:51:02]

The CQ bit was checked by borenet@google.com

[commit-bot: I haz the power, 2016-09-27 13:51:07]

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-27 13:52:29]

Description was changed from

==========
Add swarming task for upload_dm_results

BUG=skia:5719
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2360203004
==========

to

==========
Add swarming task for upload_dm_results

BUG=skia:5719
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2360203004

Committed:
https://skia.googlesource.com/skia/+/9766f0016669029fcdc49f00ceaaf66b01b03f4a
==========

[commit-bot: I haz the power, 2016-09-27 13:52:30]

Committed patchset #8 (id:140001) as
https://skia.googlesource.com/skia/+/9766f0016669029fcdc49f00ceaaf66b01b03f4a