Fully migrate gsutil module to gcloud storage#5122
Fully migrate gsutil module to gcloud storage#5122ViniciustCosta wants to merge 25 commits intomasterfrom
Conversation
ViniciustCosta
requested review from
hunsche,
javanlacerda,
jonathanmetzman and
vitaliset
| self.assertEqual(self.mock.Popen.call_args[0][0], [ | ||
| '/gsutil_path/gsutil', '-m', '-o', 'GSUtil:parallel_thread_count=16', | ||
| '-q', 'rsync', '-r', '-d', 'gs://bucket/', '/dir' | ||
| '/gcloud_path/gcloud', 'storage', '--no-user-output-enabled', '-q', |
There was a problem hiding this comment.
For the -q flag, was this placement tested ?
Also, the original command does not seem to have the equivalent of --no-user-output-enabled ?
| self.assertEqual(self.mock.Popen.call_args[0][0], [ | ||
| '/gsutil_path/gsutil', '-m', '-o', 'GSUtil:parallel_thread_count=16', | ||
| 'cp', '-I', 'gs://bucket/' | ||
| '/gcloud_path/gcloud', 'storage', '--user-output-enabled', '-q', 'cp', |
There was a problem hiding this comment.
Just curious since previous commands were --(no)-user-output-enabled, but this one is enabling it and agai with a -q option when the original gsutil command did not seem to specify that.
This is intentional and tested ?
| """Test remote rsync.""" | ||
| self.gcloud_runner_obj.rsync('gs://source_bucket/source_path', | ||
| 'gs://target_bucket/target_path') | ||
| expected_args = [ |
There was a problem hiding this comment.
The original gsutil command had the -q option as well.
ViniciustCosta
removed request for
decoNR,
hunsche,
javanlacerda,
jonathanmetzman and
vitaliset
| cp_command.append('--gzip-local-all') | ||
| cp_command.extend([file_path, _filter_path(gcs_url, write=True)]) | ||
|
|
||
| result = self.run_gcloud_storage(cp_command, timeout=timeout) |
There was a problem hiding this comment.
You might want to put some checks on the stdout/stderr prints as well.
| f'Output: {result.output}') | ||
| return False | ||
|
|
||
| # For gcloud, setting metadata is a separate step after uploading. |
There was a problem hiding this comment.
I am not perfectly sure of the use-case here, but that is not necessarily true.
You could set customer metadata along with the cp command - https://docs.cloud.google.com/sdk/gcloud/reference/storage/cp#--custom-metadata
2 participants
Fully migrate the gsutil module to use gcloud storage by:
-q, etc);Temporarily retained the
gsutil.pyfilename to facilitate revision, as changing the filename would highlight all the code and not only the actual changes (to be done in a follow-up PR). Temporarily retained theGSUTIL_PATHas a fallback, since changing this toGCLOUD_PATHwould depend on landing changes on infra init/setup scripts.Tests
All calls from the
gsutil.pymodule are already usinggcloud storagein our workflows in prod (including mac and android bots).Testing this in staging before deploying (as dev is not currently synce with master).