ssh support and a second round of abstract file operations#2
ssh support and a second round of abstract file operations#2richfitz wants to merge 30 commits intoben-gready:masterfrom
Conversation
Read and write bytes and files, directory creation, listing, etc
@ben-gready - you need to be happy with what is selected! If you'd rather something else that's totally fine. I am duplicating licence information from storr itself but you may have different opinions. Please see https://choosealicense.com if this is something you do not yet have strong opinions on
|
Tagging you here @ben-gready mostly because I don't tend to get notified about PRs on github... |
|
Wahay - finally passing - see https://travis-ci.org/richfitz/storr.remote and https://codecov.io/gh/richfitz/storr.remote/branch/ssh I'm offline from about now through until Tuesday morning |
| @@ -0,0 +1,2 @@ | |||
| YEAR: 2018 | |||
| COPYRIGHT HOLDER: Ben Gready | |||
There was a problem hiding this comment.
Thanks for adding this and the license. It's something I've basically not thought of much before. If it's acceptable to add more than one person, happy for you to be on here as well.
| ## really get what is going on. It's quite likely that what you | ||
| ## really want is | ||
| ## | ||
| ## unique(basename(files)) |
There was a problem hiding this comment.
I think you are correct, but need to test - I didn't know about the function basename!
| prefix = path_root, | ||
| max = Inf) | ||
| keys <- files_table[files_table$Size > 0,]$Key | ||
| ## TODO: this should really be stripped off the front rather |
There was a problem hiding this comment.
yeah, that sounds a lot safer!
| }, | ||
|
|
||
| destroy = function() { | ||
| ## TODO: not sure if this is right? |
There was a problem hiding this comment.
This looks correct to me. Should delete the entire store from the root. Will test when I get it working with PR#74 on storr.
|
|
||
| exists = function(path) { | ||
| path_remote <- file.path(self$root, path) | ||
| ## QUESTION: What messages are produced by this? |
There was a problem hiding this comment.
if the object does not exist, it messages the following Client error: (404) Not Found. There might be a more elegant way to test for file existence using list files etc.
| exists_dir = function(path) { | ||
| ## TODO: If it is possible to test if the remote thing is a | ||
| ## directory (e.g., one of the bits of head_object?) that would | ||
| ## be nice to get here too... |
There was a problem hiding this comment.
I think we'd need to do some logic with list files to achieve this - head_object doesn't seem to show anything along those lines. The file structure in S3 seems to be more convention than anything rigid - I can have an object named foo and another object named foo/bar (and maybe even foo/ - I'm not sure). Makes things fairly annoying!
| ## but if aws.s3 can delete multiple objects (the docs suggest | ||
| ## it can) you can probably do better. This function must | ||
| ## return a logical vector the same length as "path" | ||
| ## indicating if deletion actually happened. |
There was a problem hiding this comment.
seems like a good way to have the functionality as a stop gap - I will try to take a look at this when I get a chance.
| ## TODO: I've copied the contents of your previous function | ||
| ## here, but stripped it down to ony take the if_dir == | ||
| ## "del_only_key" path because that was what looked like what | ||
| ## was implemented. I have not tested this! |
There was a problem hiding this comment.
Looks good to me. I still need to test though!
| # storr.remote | ||
| Interface for plugging in remote storage (e.g. AWS S3) to [storr](https://github.com/richfitz/storr) | ||
|
|
||
| [](https://travis-ci.org/richfitz/storr.remote) |
There was a problem hiding this comment.
Thanks for adding the travis stuff - I have a lot to learn about all this stuff!
There was a problem hiding this comment.
Thanks so much for adding all of this. It looks great, and I think it's a massive improvement to get the generic remote_driver into storr. I'm having trouble getting this up and running for testing (still learning how to use github for PRs from other repositories), so I will come back to it probably on the weekend to try and get it setup so I'm using this branch + #74 from storr. Once I have it running, I will test using the S3 bucket I have access to, and make any changes needed to get it all working.
As far as testing the S3 driver, think we may be able to use a similar approach to how you've gone about ssh, using something like this docker image.
I'm really pressed for time these days, but I will try to get the s3 stuff to the point that it's working in the next few days, and then we can add testing etc after that.
Thanks again for all the help with this!
2 participants
This does three things:
There's a comment richfitz/storr#72 (comment) that describes a process for resolving all the live work in a sensible way