Uh oh!
There was an error while loading. Please reload this page.
ARROW-9868: [C++][R] Provide CopyFiles for copying files between FileSystems - #8101
ARROW-9868: [C++][R] Provide CopyFiles for copying files between FileSystems#8101bkietz wants to merge 4 commits into
Conversation
nealrichardson
left a comment
There was a problem hiding this comment.
Thanks for doing this! A couple of minor notes
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
We may introduce a FileLocator struct or something that would be a filesystem, path pair. Would also be useful with FromURI.
There was a problem hiding this comment.
Would you then want the signature to be Status CopyFiles(const std::vector<FileLocator>& sources, const std::vector<FileLocator>& destinations, ...)?
There was a problem hiding this comment.
That sounds ok, assuming having potentially different filesystems in those vectors is not a problem.
There was a problem hiding this comment.
I've wanted some kind of object to contain fs and path (like what you get from FileSystemFromUri) for a while too. Yesterday in fact I was experimenting with using SubTreeFileSystem to hold that, since it initializes with a fs and path. That mostly works as expected (for the operations I was trying at least).
jorisvandenbossche
left a comment
There was a problem hiding this comment.
Python bindings are always welcome as well ;) (but I can also write them, here or in a follow-up PR)
One thing I was wondering API wise: right now you need to pass a list of file paths, right? As a user it would also be nice to specify a directory to copy. Would that also be possible? (or with a selector?)
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
nealrichardson
left a comment
There was a problem hiding this comment.
This is ok with me. I can follow up separately with R implementation and tests (on top of the function added here). I am curious to see some thought around ARROW-9909 but don't think that needs to hold this.
Rebase should hopefully resolve the R Windows CI failure.
9f2a158 to
a0c14f5Comparenealrichardson
commented
Sep 10, 2020
@bkietz is this done? |
No description provided.