PyFilesystem / pyfilesystem

Python filesystem abstraction layer
http://pyfilesystem.org/
BSD 3-Clause "New" or "Revised" License
287 stars 63 forks source link

Support alternate datetime format for S3 files #262

Open mattharrison opened 7 years ago

mattharrison commented 7 years ago

This supports alternate formats for S3 buckets

lurch commented 7 years ago

I wonder if using a regex might be more reliable than simply looking for a hyphen? shrug

lurch commented 7 years ago

The second of these two commits seems to be a duplicate of #232 ?

I've never used S3, so can't test this myself, and I'd rather have feedback from other users that this still works with all versions of Python, before merging.

mattharrison commented 7 years ago

@lurch - Thanks for your comments. I'm not sure how the second commit got into this PR.

My Py3 patch was based on that one, so they are similar.

Regarding using a regex. My opinion is that a regex is a tool to use when you have nothing else. In this case I know the other format doesn't have a dash in it, so a simple check should suffix. I guess the more correct code would be to use a smarter date parser (would introduce 3rd party dep) or try and format, then in the error handling code try again.

Anyway, it works for me in Python 3 for adding, removing, renaming, and getting file info of both binary and text files, as well as directories. shrug :)

lurch commented 7 years ago

If you can remove the second commit from this PR and force-push, I'd be happy to merge this alternate-datetime PR.

I suspect the second commit snuck into this PR because you've created it from your master branch, rather than the usual practice of PR-ing from an alternate branch.