Repository navigation
feat: Implement copy and rename - #14
Conversation
db729c2 to
4c097b8
Compare
|
@wjones127 - I'd be happy to pick up Azure support. |
4c097b8 to
bb0b0c0
Compare
| } | ||
| } | ||
|
|
||
| #[cfg(windows)] |
There was a problem hiding this comment.
I wonder if we could use https://doc.rust-lang.org/std/fs/fn.hard_link.html instead? I think this should be atomic on all sane platforms, and avoids a lot of non-trivial code? This would have the obvious caveat that it wouldn't allow cross-filesystem copies, but I'm inclined to think that doesn't really matter...
Edit: In fact the renameat function used below will error with EXDEV if used across mount points anyway, so this caveat doesn't apply
There was a problem hiding this comment.
Sure, I'll try that.
| Ok(()) | ||
| } | ||
|
|
||
| async fn rename_no_replace(&self, _source: &Path, _dest: &Path) -> Result<()> { |
There was a problem hiding this comment.
As mentioned below, I wonder if rather than a rename_no_replace, which no object stores actually support, if copy_if_not_exists would be sufficient?
There was a problem hiding this comment.
Yeah that makes sense. copy_if_not_exist is sufficient for delta-rs.
bb0b0c0 to
28d4ae9
Compare
28d4ae9 to
16f0c0c
Compare
tustvold
left a comment
tustvold
left a comment
There was a problem hiding this comment.
This is stellar work, I left some minor suggestions, e.g. moving todo!() to return NotImplemented errors, but otherwise this looks good to go 😄
|
I have a draft ready for the azure support. Once this gets merged, I'll rebase and open a new PR. |
tustvold
added
kodiak: merge.method = 'squash'
|
Hmm... I think CI should kick in if you push another commit, e.g. merge master |
|
@tustvold I've signed the CLA 👍 |
tustvold
added
kodiak: merge.method = 'squash'
|
Thanks again 🎉 |
3 participants
Based on discussion in #10, we may wish to do this instead of #11.
Adds three new methods to the
ObjectStoretrait:copy(from, to)rename(from, to)(with default implementation)rename_no_replace(from, to)This doesn't implement them for all stores, but here is what I have done:
copy()rename()rename_no_replace()(Added better Windows support)copy()rename()rename_no_replace()copy()rename()(default copy and then delete)rename_no_replace()(will need to use dynamodb-lock)copy()rename()rename_no_replace()copy()rename()(default copy and then delete)rename_no_replace()(GCS supports this, but need to add precondition support to cloud-storage-rs)