search

PyFilesystem / pyfilesystem

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

allow any s3fs connection args #233

Closed aisch closed 8 years ago

aisch commented 8 years ago

so we can use boto to deduce all this from env rather than hardcoding.

should still work w/ previous calls except that: cxn failure due to missing creds is delayed to first S3FS._s3conn, if you like i can update pr to try cxn once in S3FS.__init__ and convert missing creds error to CreateFailedError there.

closes #211

aisch commented 8 years ago

@lurch any interest in merging this pr?

lurch commented 8 years ago

I'm afraid I've never even looked at any of the S3FS code - that's more @willmcgugan 's or @rfk 's area.

aisch commented 8 years ago

@willmcgugan @rfk do these S3FS changes look ok to merge?

jimbocoder commented 8 years ago

We'd love to see this merged if there are no blockers! Looks good to me, but I don't maintain this project :)

lurch commented 8 years ago

Well I've never used any S3 stuff, so I haven't tested this PR. And even though @willmcgugan or @rfk haven't provided any feedback, this change looks okay from a quick glance, so I'll go ahead and merge it anyway, and hope it doesn't break anything ;-)

jimbocoder commented 8 years ago

We've taken this all the way from dev through to production and it's working great for 3 days now without issue.

@lurch I'm curious about a practical aspect. I notice this change is not in pypi even though it's present in the master branch here. Is there some periodic update to pypi for this project, or a testing process that needs to happen first, or what? Currently we've forked this repo, but usually vanilla pypi would be way nicer :)

Thanks for merging this!

lurch commented 8 years ago

Thanks for the positive feedback :+1:

pypi releases are entirely managed by @willmcgugan (the creator and owner of this project), and out of my hands.

aisch commented 8 years ago

@jimbocoder @lurch thx for getting this merged :tada:

jimbocoder commented 8 years ago

@willmcgugan After a couple months now, we've put tens of millions of files through this change, with no adverse effects to report! Would love to see this one make it into the pypi index, so we can "unfork" the pypi release! Thanks!