Skip to content

allow any s3fs connection args#233

Merged
lurch merged 1 commit intoPyFilesystem:masterfrom
aisch:s3fs-conn-args
Sep 7, 2016
Merged

allow any s3fs connection args#233
lurch merged 1 commit intoPyFilesystem:masterfrom
aisch:s3fs-conn-args

Conversation

@aisch
Copy link
Copy Markdown

@aisch aisch commented Jan 6, 2016

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
Copy link
Copy Markdown
Author

aisch commented Mar 3, 2016

@lurch any interest in merging this pr?

@lurch
Copy link
Copy Markdown
Contributor

lurch commented Mar 3, 2016

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

@aisch
Copy link
Copy Markdown
Author

aisch commented Jul 27, 2016

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

@jimbocoder
Copy link
Copy Markdown

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

@lurch
Copy link
Copy Markdown
Contributor

lurch commented Sep 7, 2016

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 ;-)

@lurch lurch merged commit fc74df6 into PyFilesystem:master Sep 7, 2016
@jimbocoder
Copy link
Copy Markdown

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
Copy link
Copy Markdown
Contributor

lurch commented Sep 16, 2016

Thanks for the positive feedback 👍

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

@aisch
Copy link
Copy Markdown
Author

aisch commented Sep 17, 2016

@jimbocoder @lurch thx for getting this merged 🎉

@jimbocoder
Copy link
Copy Markdown

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AWS Credentials for S3 shouldn't require environment variables

3 participants