From e48b81e62bfd159d0b3dfdf63c56bd9a76578d0f Mon Sep 17 00:00:00 2001 From: "primiano@chromium.org" Date: Tue, 17 Feb 2015 12:33:35 +0000 Subject: [PATCH] [download_from_google_storage] Don't list ALL objects to check for ACLs Currently check_bucket_permissions() in download_from_google_storage.py performs a gsutil ls gs://bucket to determine whether the user has access to the bucket or not. This can be an EXTREMELY expensive operation (~minutes) if the bucket in question has a lot of objects in the root (real case: chrome-telemetry). It is worth noting that check_bucket_permissions() is not called just for uploads but also for downloads, hence this is slowing down all invocations of gclient sync on users and bots machine. Removing the check_bucket_permissions() and let gsutil fail with an Unauthorized error message if ACLs are not met. BUG=458059 Review URL: https://codereview.chromium.org/923473002 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@294083 0039d316-1c4b-4281-b951-d872f2087c98 --- download_from_google_storage.py | 18 ------------------ upload_to_google_storage.py | 6 ------ 2 files changed, 24 deletions(-) diff --git a/download_from_google_storage.py b/download_from_google_storage.py index 0f53ea46e..5c1df0324 100755 --- a/download_from_google_storage.py +++ b/download_from_google_storage.py @@ -105,18 +105,6 @@ class Gsutil(object): return (code, out, err) -def check_bucket_permissions(base_url, gsutil): - code, _, ls_err = gsutil.check_call('ls', base_url) - if code != 0: - print >> sys.stderr, ls_err - if code == 403: - print >> sys.stderr, 'Got error 403 while authenticating to %s.' % base_url - print >> sys.stderr, 'Try running "download_from_google_storage --config".' - elif code == 404: - print >> sys.stderr, '%s not found.' % base_url - return code - - def check_platform(target): """Checks if any parent directory of target matches (win|mac|linux).""" assert os.path.isabs(target) @@ -454,12 +442,6 @@ def main(args): base_url = 'gs://%s' % options.bucket - # Check we have a valid bucket with valid permissions. - if not options.no_auth: - code = check_bucket_permissions(base_url, gsutil) - if code: - return code - return download_from_google_storage( input_filename, base_url, gsutil, options.num_threads, options.directory, options.recursive, options.force, options.output, options.ignore_errors, diff --git a/upload_to_google_storage.py b/upload_to_google_storage.py index 75db4174e..e81e0198f 100755 --- a/upload_to_google_storage.py +++ b/upload_to_google_storage.py @@ -15,7 +15,6 @@ import sys import threading import time -from download_from_google_storage import check_bucket_permissions from download_from_google_storage import get_sha1 from download_from_google_storage import Gsutil from download_from_google_storage import printer_worker @@ -243,11 +242,6 @@ def main(args): base_url = 'gs://%s' % options.bucket - # Check we have a valid bucket with valid permissions. - code = check_bucket_permissions(base_url, gsutil) - if code: - return code - return upload_to_google_storage( input_filenames, base_url, gsutil, options.force, options.use_md5, options.num_threads, options.skip_hashing)