add ACL-setting code to upload_rendered_skps.py

slave/skia_slave_scripts/build_step.py

Index: slave/skia_slave_scripts/build_step.py
diff --git a/slave/skia_slave_scripts/build_step.py b/slave/skia_slave_scripts/build_step.py
index 9a821de5b0b1112d13de5dbe897f959338377eb5..7c702a1e45efe6582d3e7e172804a1f08e05d58a 100644
--- a/slave/skia_slave_scripts/build_step.py
+++ b/slave/skia_slave_scripts/build_step.py
@@ -33,6 +33,7 @@ import builder_name_schema
 import slave_hosts_cfg
 import slaves_cfg
 
+from py.utils import gs_utils
 from py.utils import misc
 
 
@@ -51,8 +52,14 @@ INT_FALSE = 0
 build_step_stdout_has_written = multiprocessing.Value('i', INT_FALSE)
 
 
-# The canned acl to use while copying playback files to Google Storage.
-PLAYBACK_CANNED_ACL = 'private'
+# The canned acl to use while copying playback (SKP) files to Google Storage.
+# They should not be world-readable!
+PLAYBACK_CANNED_ACL = gs_utils.PREDEFINED_ACL_PRIVATE
+PLAYBACK_FINEGRAINED_ACL_LIST = [
+    (gs_utils.ID_TYPE_GROUP_BY_DOMAIN, 'google.com', gs_utils.PERMISSION_READ),
+]

[borenet, 2014/07/18 22:11:12]

I would strongly prefer that these go into gs_utils instead of build_step.

[epoger, 2014/07/21 13:42:51]

Given that the buildbots write SKP files into Google Storage, but individual
developers (i.e. users of the Skia repo) never do, it seems like these
configurations should live within the buildbot repo, not common.

So I can think of 3 options.  I will happily do any of them; you tell me which
one you want:
1. Move these constants into gs_utils.py in the common repo.
2. Define them within upload_rendered_skps.py, and any other build steps that
need them import them from there.
3. Define them within a new shared module in the buildbot repo
("skp_playback.py"?)
4. Something else you come up with

Also, would you like me to leave BOTO_CREDENTIALS_FILE defined here or move it
elsewhere?  (Again, I think it's buildbot-specific, not appropriate for the
common repo)

[borenet, 2014/07/21 14:07:46]

I like #1 and #3 (though I really don't like the "playback" term; it confuses me
for some reason), and possibly another option would be to create a new
gs_utils.py in skia-buildbot which just imports * from gs_utils in common repo
and adds whatever buildbot-specific stuff we want.

And really, it's not a crime to have variables shared by multiple build steps in
this file.  I just get nervous every time this file gets touched because of the
potential for disaster.

[epoger, 2014/07/21 20:01:31]

Cool idea.  I went with it... in order to do it without "import * from gs_utils"
(which is considered dangerous) I had to modify the structure of common's
gs_utils.py in https://codereview.chromium.org/405083005/ ('use better mechanism
for enums in gs_utils.py')

+BOTO_CREDENTIALS_FILE = os.path.join(
+    BUILDBOT_PATH, 'third_party', 'chromium_buildbot', 'site_config', '.boto')

[epoger, 2014/07/18 21:10:24]

This is where we already put the .boto file on the bots... we can change this
path if we want, once old_gs_utils.py is gone.  (Unless we need the
chromium_buildbot code to use these same credentials???)

[borenet, 2014/07/21 14:07:46]

If it's okay with you, let's leave it as-is, since it'll need to be there
eventually when we're in sync with upstream code.

[epoger, 2014/07/21 20:01:31]

Now, users of gs_utils.py within buildbot automatically get the credentials from
this filepath, unless they ask for something different.

 
 
 class BuildStepWarning(Exception):