Implemented follow-symlinks feature for S3. #854
Conversation
Added unit and integration tests along with PEP8 reformatting for the entire file.
| "are followed only when uploading locally to S3. Note that S3 " \ | ||
| "does not support symbolic links so contents of linked files are " \ | ||
| "uploaded under name of symbolic file." | ||
|
|
There was a problem hiding this comment.
Re-wording suggestion:
Symbolic links are followed only when uploading to S3 from the local filesystem. Note that S3 does not support symbolic links, so the contents of the link target are uploaded under the name of the link.
|
Looks pretty good! Just a few comments above 👍 |
|
This is a change of the existing behavior. While I agree that ignoring symlinks by default makes sense, we can't change the existing behavior. Let's add two options, suggested in #453, a --follow-symlink and a --no-follow-symlink option and leave the default option to --follow-symlink. Another thing to keep in mind, if you're doing major code formatting cleanup, try to keep those in separate commits. It will make it easier for a reviewer to see the actual functional changes vs. the non functional changes. |
|
Just added --[no]-follow-symlinks options. @danielgtaylor @jamesls |
| @@ -147,6 +152,23 @@ def _check_paths_decoded(self, path, names): | |||
| if not isinstance(name, six.text_type): | |||
| raise FileDecodingError(path, name) | |||
|
|
|||
| def check_ignore_file(self, path): | |||
There was a problem hiding this comment.
given this is a function that returns true/false, a name like should_ignore_file is easier to follow.
|
Reverted handling of broken symlinks to original behavior (Fails on attempt to follow broken symlink). Edited code based on feedback. @danielgtaylor @jamesls |
|
|
Looks good.
|
LGTM as well. 🚢-it! |
Added unit and integration tests along with PEP8 reformatting for
the entire file.
Fixes:
#453
#781