Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various small changes #42

Merged
merged 2 commits into from
May 13, 2016
Merged

Various small changes #42

merged 2 commits into from
May 13, 2016

Conversation

mrocklin
Copy link
Collaborator

  1. Create a directory for mapping if it doesn't exist.
  2. Include original exception error messages

@martindurant
Copy link
Member

S3 keys don't really need a "directory key" to live within. The empty key wouldn't hurt, I suppose.

@martindurant
Copy link
Member

The problem is with recursive rm: if there are not actually any files below to pass to bulk_delete.

@mrocklin
Copy link
Collaborator Author

I ran into this when creating a mapping from a non-existent bucket, in which case the mkdir does become necessary (I think).

@martindurant
Copy link
Member

Ah, so it's not the directory you want to create per se, but the bucket for it to live within. I'm not sure we should do that automatically.

@mrocklin
Copy link
Collaborator Author

What would you recommend? Improved error message?

@martindurant
Copy link
Member

Error message I think, yes. Either happens at check time or on first write when check=False.

@mrocklin
Copy link
Collaborator Author

Thoughts on adding a create=False keyword argument to create the bucket if necessary and having the error message point them towards that keyword argument? This seems significantly more convenient and helps mapping users (e.g. zarr users) from having to be exposed to the S3FileSystem API.

@martindurant
Copy link
Member

OK, fine with me. The whole mapping thing of course is for zarr's convenience. Most users of s3fs itself probably won't touch it.

@mrocklin mrocklin merged commit a43d899 into master May 13, 2016
@mrocklin mrocklin deleted the various branch May 13, 2016 18:06
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.

2 participants