|
|
Created:
5 years, 6 months ago by Mike Schwartz Modified:
5 years, 6 months ago CC:
thobrla, gsutil-crs_google.com Visibility:
Public. |
DescriptionAdd documentation advising against using gsutil rsync or cp over directories containing symlinks
Patch Set 1 #
Total comments: 4
MessagesTotal messages: 6
Motivated by discussion around https://github.com/GoogleCloudPlatform/gsutil/issues/222 (see https://groups.google.com/a/google.com/d/msg/gsutil-team/kJ1Bk4WWAHw/k-QtJz_q...)
Sign in to reply to this message.
https://codereview.appspot.com/342260043/diff/1/gslib/commands/rsync.py File gslib/commands/rsync.py (right): https://codereview.appspot.com/342260043/diff/1/gslib/commands/rsync.py#newco... gslib/commands/rsync.py:224: It's unlikely gsutil rsync will ever support symlinks, because doing so This sounds a bit defensive on our part. I'd suggest something like the following after the bullet points (though I don't know if it fully conveys the existing text): " Since gsutil is meant to support data operations - like moving a data set to the cloud for large scale computational processing - and also needs to be compatible both in the Cloud and across common operating systems, there are no plans for gsutil rsync to support symlinks. We strongly recommend that customers either not use gstil rsync over directories containing symlinks, or use the the -e option to exclude symlinks. "
Sign in to reply to this message.
https://codereview.appspot.com/342260043/diff/1/gslib/commands/rsync.py File gslib/commands/rsync.py (right): https://codereview.appspot.com/342260043/diff/1/gslib/commands/rsync.py#newco... gslib/commands/rsync.py:224: It's unlikely gsutil rsync will ever support symlinks, because doing so On 2018/09/21 22:49:35, nstock wrote: > This sounds a bit defensive on our part. I'd suggest something like the > following after the bullet points (though I don't know if it fully conveys the > existing text): > > " > Since gsutil is meant to support data operations - like moving a data set to the > cloud for large scale computational processing - and also needs to be compatible > both in the Cloud and across common operating systems, there are no plans for > gsutil rsync to support symlinks. > > We strongly recommend that customers either not use gstil rsync over > directories containing symlinks, or use the the -e option to exclude symlinks. > " I agree with tweaking the wording to sound less like a justification and more like guidance. Nit: be consistent about "Cloud" capitalization, and replace "customers" with "users".
Sign in to reply to this message.
https://codereview.appspot.com/342260043/diff/1/gslib/commands/rsync.py File gslib/commands/rsync.py (right): https://codereview.appspot.com/342260043/diff/1/gslib/commands/rsync.py#newco... gslib/commands/rsync.py:224: It's unlikely gsutil rsync will ever support symlinks, because doing so On 2018/09/21 22:49:35, nstock wrote: > This sounds a bit defensive on our part. I'd suggest something like the > following after the bullet points (though I don't know if it fully conveys the > existing text): > > " > Since gsutil is meant to support data operations - like moving a data set to the > cloud for large scale computational processing - and also needs to be compatible > both in the Cloud and across common operating systems, there are no plans for > gsutil rsync to support symlinks. > > We strongly recommend that customers either not use gstil rsync over > directories containing symlinks, or use the the -e option to exclude symlinks. > " Done. https://codereview.appspot.com/342260043/diff/1/gslib/commands/rsync.py#newco... gslib/commands/rsync.py:224: It's unlikely gsutil rsync will ever support symlinks, because doing so On 2018/09/21 23:37:44, houglum wrote: > On 2018/09/21 22:49:35, nstock wrote: > > This sounds a bit defensive on our part. I'd suggest something like the > > following after the bullet points (though I don't know if it fully conveys the > > existing text): > > > > " > > Since gsutil is meant to support data operations - like moving a data set to > the > > cloud for large scale computational processing - and also needs to be > compatible > > both in the Cloud and across common operating systems, there are no plans for > > gsutil rsync to support symlinks. > > > > We strongly recommend that customers either not use gstil rsync over > > directories containing symlinks, or use the the -e option to exclude symlinks. > > " > > I agree with tweaking the wording to sound less like a justification and more > like guidance. > > Nit: be consistent about "Cloud" capitalization, and replace "customers" with > "users". I used lowercase throughout, since "cloud" isn't a proper noun (vs Google Cloud Storage, which is).
Sign in to reply to this message.
FYI when I ran upload.py I used the wrong --rev commit pointer once, so the diffs don't make it clear all the changes I made even though the revised text is present. So please just read the full text of the new section again. On Sat, Sep 22, 2018 at 2:01 PM, <mfschwartz@google.com> wrote: > > https://codereview.appspot.com/342260043/diff/1/gslib/commands/rsync.py > File gslib/commands/rsync.py (right): > > https://codereview.appspot.com/342260043/diff/1/gslib/comman > ds/rsync.py#newcode224 > gslib/commands/rsync.py:224: It's unlikely gsutil rsync will ever > support symlinks, because doing so > On 2018/09/21 22:49:35, nstock wrote: > >> This sounds a bit defensive on our part. I'd suggest something like >> > the > >> following after the bullet points (though I don't know if it fully >> > conveys the > >> existing text): >> > > " >> Since gsutil is meant to support data operations - like moving a data >> > set to the > >> cloud for large scale computational processing - and also needs to be >> > compatible > >> both in the Cloud and across common operating systems, there are no >> > plans for > >> gsutil rsync to support symlinks. >> > > We strongly recommend that customers either not use gstil rsync over >> directories containing symlinks, or use the the -e option to exclude >> > symlinks. > >> " >> > > Done. > > https://codereview.appspot.com/342260043/diff/1/gslib/comman > ds/rsync.py#newcode224 > gslib/commands/rsync.py:224: It's unlikely gsutil rsync will ever > support symlinks, because doing so > On 2018/09/21 23:37:44, houglum wrote: > >> On 2018/09/21 22:49:35, nstock wrote: >> > This sounds a bit defensive on our part. I'd suggest something like >> > the > >> > following after the bullet points (though I don't know if it fully >> > conveys the > >> > existing text): >> > >> > " >> > Since gsutil is meant to support data operations - like moving a >> > data set to > >> the >> > cloud for large scale computational processing - and also needs to >> > be > >> compatible >> > both in the Cloud and across common operating systems, there are no >> > plans for > >> > gsutil rsync to support symlinks. >> > >> > We strongly recommend that customers either not use gstil rsync over >> > directories containing symlinks, or use the the -e option to exclude >> > symlinks. > >> > " >> > > I agree with tweaking the wording to sound less like a justification >> > and more > >> like guidance. >> > > Nit: be consistent about "Cloud" capitalization, and replace >> > "customers" with > >> "users". >> > > I used lowercase throughout, since "cloud" isn't a proper noun (vs > Google Cloud Storage, which is). > > https://codereview.appspot.com/342260043/ >
Sign in to reply to this message.
I uploaded another diffbase, this time using the correct --rev commit pointer, and now it's showing the old review content. So I'm closing that rietveld review and opening a new one ( http://codereview.appspot.com/353780043) On Sat, Sep 22, 2018 at 2:07 PM, Michael Schwartz <mfschwartz@google.com> wrote: > FYI when I ran upload.py I used the wrong --rev commit pointer once, so > the diffs don't make it clear all the changes I made even though the > revised text is present. So please just read the full text of the new > section again. > > On Sat, Sep 22, 2018 at 2:01 PM, <mfschwartz@google.com> wrote: > >> >> https://codereview.appspot.com/342260043/diff/1/gslib/commands/rsync.py >> File gslib/commands/rsync.py (right): >> >> https://codereview.appspot.com/342260043/diff/1/gslib/comman >> ds/rsync.py#newcode224 >> gslib/commands/rsync.py:224: It's unlikely gsutil rsync will ever >> support symlinks, because doing so >> On 2018/09/21 22:49:35, nstock wrote: >> >>> This sounds a bit defensive on our part. I'd suggest something like >>> >> the >> >>> following after the bullet points (though I don't know if it fully >>> >> conveys the >> >>> existing text): >>> >> >> " >>> Since gsutil is meant to support data operations - like moving a data >>> >> set to the >> >>> cloud for large scale computational processing - and also needs to be >>> >> compatible >> >>> both in the Cloud and across common operating systems, there are no >>> >> plans for >> >>> gsutil rsync to support symlinks. >>> >> >> We strongly recommend that customers either not use gstil rsync over >>> directories containing symlinks, or use the the -e option to exclude >>> >> symlinks. >> >>> " >>> >> >> Done. >> >> https://codereview.appspot.com/342260043/diff/1/gslib/comman >> ds/rsync.py#newcode224 >> gslib/commands/rsync.py:224: It's unlikely gsutil rsync will ever >> support symlinks, because doing so >> On 2018/09/21 23:37:44, houglum wrote: >> >>> On 2018/09/21 22:49:35, nstock wrote: >>> > This sounds a bit defensive on our part. I'd suggest something like >>> >> the >> >>> > following after the bullet points (though I don't know if it fully >>> >> conveys the >> >>> > existing text): >>> > >>> > " >>> > Since gsutil is meant to support data operations - like moving a >>> >> data set to >> >>> the >>> > cloud for large scale computational processing - and also needs to >>> >> be >> >>> compatible >>> > both in the Cloud and across common operating systems, there are no >>> >> plans for >> >>> > gsutil rsync to support symlinks. >>> > >>> > We strongly recommend that customers either not use gstil rsync over >>> > directories containing symlinks, or use the the -e option to exclude >>> >> symlinks. >> >>> > " >>> >> >> I agree with tweaking the wording to sound less like a justification >>> >> and more >> >>> like guidance. >>> >> >> Nit: be consistent about "Cloud" capitalization, and replace >>> >> "customers" with >> >>> "users". >>> >> >> I used lowercase throughout, since "cloud" isn't a proper noun (vs >> Google Cloud Storage, which is). >> >> https://codereview.appspot.com/342260043/ >> > >
Sign in to reply to this message.
|