From 50c631e318c698bcd0c8cd19e326e5aa4d6b0369 Mon Sep 17 00:00:00 2001 From: Joanna Wang Date: Fri, 13 May 2022 19:26:01 +0000 Subject: [PATCH] Skip 'ls' when downloading from gs. Bug: 959167 Change-Id: I160658264dac71a9cff280cf3ba606d583d8f2b3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3646593 Reviewed-by: Gavin Mak Commit-Queue: Joanna Wang --- download_from_google_storage.py | 33 ++++++++----------- .../download_from_google_storage_unittest.py | 24 ++++---------- 2 files changed, 20 insertions(+), 37 deletions(-) diff --git a/download_from_google_storage.py b/download_from_google_storage.py index 52ef97cd1..58d4913cb 100755 --- a/download_from_google_storage.py +++ b/download_from_google_storage.py @@ -273,9 +273,20 @@ def _downloader_worker_thread(thread_num, q, force, base_url, skip = False if skip: continue - # Check if file exists. + file_url = '%s/%s' % (base_url, input_sha1_sum) - (code, _, err) = gsutil.check_call('ls', file_url) + + try: + if delete: + os.remove(output_filename) # Delete the file if it exists already. + except OSError: + if os.path.exists(output_filename): + out_q.put('%d> Warning: deleting %s failed.' % ( + thread_num, output_filename)) + if verbose: + out_q.put('%d> Downloading %s@%s...' % ( + thread_num, output_filename, input_sha1_sum)) + code, _, err = gsutil.check_call('cp', file_url, output_filename) if code != 0: if code == 404: out_q.put('%d> File %s for %s does not exist, skipping.' % ( @@ -294,25 +305,9 @@ def _downloader_worker_thread(thread_num, q, force, base_url, # Other error, probably auth related (bad ~/.boto, etc). out_q.put('%d> Failed to fetch file %s for %s, skipping. [Err: %s]' % (thread_num, file_url, output_filename, err)) - ret_codes.put((1, 'Failed to fetch file %s for %s. [Err: %s]' % + ret_codes.put((code, 'Failed to fetch file %s for %s. [Err: %s]' % (file_url, output_filename, err))) continue - # Fetch the file. - if verbose: - out_q.put('%d> Downloading %s@%s...' % - (thread_num, output_filename, input_sha1_sum)) - try: - if delete: - os.remove(output_filename) # Delete the file if it exists already. - except OSError: - if os.path.exists(output_filename): - out_q.put('%d> Warning: deleting %s failed.' % ( - thread_num, output_filename)) - code, _, err = gsutil.check_call('cp', file_url, output_filename) - if code != 0: - out_q.put('%d> %s' % (thread_num, err)) - ret_codes.put((code, err)) - continue remote_sha1 = get_sha1(output_filename) if remote_sha1 != input_sha1_sum: diff --git a/tests/download_from_google_storage_unittest.py b/tests/download_from_google_storage_unittest.py index eb3ab32bf..017fc59fa 100755 --- a/tests/download_from_google_storage_unittest.py +++ b/tests/download_from_google_storage_unittest.py @@ -248,7 +248,6 @@ class DownloadTests(unittest.TestCase): sha1_hash = self.lorem_ipsum_sha1 input_filename = '%s/%s' % (self.base_url, sha1_hash) output_filename = os.path.join(self.base_path, 'uploaded_lorem_ipsum.txt') - self.gsutil.add_expected(0, '', '') # ls self.gsutil.add_expected(0, '', '', lambda: shutil.copyfile( self.lorem_ipsum, output_filename)) # cp self.queue.put((sha1_hash, output_filename)) @@ -258,8 +257,6 @@ class DownloadTests(unittest.TestCase): 0, self.queue, False, self.base_url, self.gsutil, stdout_queue, self.ret_codes, True, False) expected_calls = [ - ('check_call', - ('ls', input_filename)), ('check_call', ('cp', input_filename, output_filename))] sha1_hash = '7871c8e24da15bad8b0be2c36edc9dc77e37727f' @@ -304,8 +301,6 @@ class DownloadTests(unittest.TestCase): 0, self.queue, True, self.base_url, self.gsutil, stdout_queue, self.ret_codes, True, True, delete=False) expected_calls = [ - ('check_call', - ('ls', input_filename)), ('check_call', ('cp', input_filename, output_filename))] if sys.platform != 'win32': @@ -362,8 +357,7 @@ class DownloadTests(unittest.TestCase): True, True, delete=False) - expected_calls += [('check_call', ('ls', input_filename)), - ('check_call', ('cp', input_filename, output_filename))] + expected_calls += [('check_call', ('cp', input_filename, output_filename))] if sys.platform != 'win32': expected_calls.append( ('check_call', ('stat', 'gs://sometesturl/%s' % sha1_hash))) @@ -389,17 +383,18 @@ class DownloadTests(unittest.TestCase): self.queue.put((sha1_hash, output_filename)) self.queue.put((None, None)) stdout_queue = queue.Queue() - self.gsutil.add_expected(1, '', '') # Return error when 'ls' is called. + self.gsutil.add_expected(1, '', '') # Return error when 'cp' is called. download_from_google_storage._downloader_worker_thread( 0, self.queue, False, self.base_url, self.gsutil, stdout_queue, self.ret_codes, True, False) expected_output = [ + '0> Downloading %s@%s...' % (output_filename, sha1_hash), '0> Failed to fetch file %s for %s, skipping. [Err: ]' % ( input_filename, output_filename), ] expected_calls = [ ('check_call', - ('ls', input_filename)) + ('cp', input_filename, output_filename)) ] expected_ret_codes = [ (1, 'Failed to fetch file %s for %s. [Err: ]' % ( @@ -413,8 +408,7 @@ class DownloadTests(unittest.TestCase): sha1_hash = '7871c8e24da15bad8b0be2c36edc9dc77e37727f' input_filename = '%s/%s' % (self.base_url, sha1_hash) output_filename = os.path.join(self.base_path, 'uploaded_lorem_ipsum.txt') - self.gsutil.add_expected(0, '', '') # ls - self.gsutil.add_expected(101, '', 'Test error message.') + self.gsutil.add_expected(101, '', 'Test error message.') # cp code = download_from_google_storage.download_from_google_storage( input_filename=sha1_hash, base_url=self.base_url, @@ -430,8 +424,6 @@ class DownloadTests(unittest.TestCase): auto_platform=False, extract=False) expected_calls = [ - ('check_call', - ('ls', input_filename)), ('check_call', ('cp', input_filename, output_filename)) ] @@ -450,8 +442,7 @@ class DownloadTests(unittest.TestCase): def _write_bad_file(): with open(output_filename, 'w') as f: f.write('foobar') - self.gsutil.add_expected(0, '', '') - self.gsutil.add_expected(0, '', '', _write_bad_file) + self.gsutil.add_expected(0, '', '', _write_bad_file) # cp download_from_google_storage._downloader_worker_thread( 1, q, True, self.base_url, self.gsutil, out_q, ret_codes, True, False) self.assertTrue(q.empty()) @@ -470,7 +461,6 @@ class DownloadTests(unittest.TestCase): input_filename = '%s/%s' % (self.base_url, sha1_hash) output_filename = os.path.join(self.base_path, 'uploaded_lorem_ipsum.txt') self.gsutil.add_expected(0, '', '') # version - self.gsutil.add_expected(0, '', '') # ls self.gsutil.add_expected(0, '', '', lambda: shutil.copyfile( self.lorem_ipsum, output_filename)) # cp code = download_from_google_storage.download_from_google_storage( @@ -489,8 +479,6 @@ class DownloadTests(unittest.TestCase): extract=False) expected_calls = [ ('check_call', ('version',)), - ('check_call', - ('ls', input_filename)), ('check_call', ('cp', input_filename, output_filename))] if sys.platform != 'win32':