crepererum commented on code in PR #899:
URL:
https://github.com/apache/arrow-rs-object-store/pull/899#discussion_r4452773314
##########
src/upload.rs:
##########
@@ -29,6 +29,36 @@ use tokio::task::JoinSet;
/// An upload part request
pub type UploadPart = BoxFuture<'static, Result<()>>;
+#[derive(Debug, Clone, Copy, Default)]
+pub(crate) enum MultipartUploadMode {
+ #[default]
+ Overwrite,
+ Create,
+}
+
+impl TryFrom<PutMode> for MultipartUploadMode {
+ type Error = Error;
+
+ fn try_from(mode: PutMode) -> Result<Self> {
+ match mode {
+ PutMode::Overwrite => Ok(Self::Overwrite),
+ PutMode::Create => Ok(Self::Create),
+ PutMode::Update(_) => Err(Error::NotSupported {
Review Comment:
why?
##########
src/upload.rs:
##########
@@ -68,12 +98,46 @@ pub trait MultipartUpload: Send + std::fmt::Debug {
/// [R2]:
https://developers.cloudflare.com/r2/objects/multipart-objects/#limitations
fn put_part(&mut self, data: PutPayload) -> UploadPart;
- /// Complete the multipart upload
+ /// Select the mode used by [`Self::complete`].
+ ///
+ /// Uploads default to [`PutMode::Overwrite`]. The mode can be changed
before
+ /// completion, including after uploading parts. An error leaves the
previous
+ /// mode unchanged; callers must handle it before completing the upload.
+ ///
+ /// [`PutMode::Create`] requires an atomic existence check and publication.
+ /// Unsupported modes return [`Error::NotSupported`] instead of silently
+ /// selecting overwrite. The default implementation accepts only overwrite.
+ ///
+ /// ```
+ /// # use object_store::{memory::InMemory, ObjectStoreExt, PutMode};
+ /// # fn main() -> object_store::Result<()> {
futures_executor::block_on(async {
+ /// let store = InMemory::new();
+ /// let mut upload = store.put_multipart(&"new-object".into()).await?;
+ /// upload.put_part("data".into()).await?;
+ /// upload.set_put_mode(PutMode::Create)?;
+ /// upload.complete().await?;
+ /// # Ok(()) }) }
+ /// ```
+ fn set_put_mode(&mut self, mode: PutMode) -> Result<()> {
Review Comment:
So is the intention to set this after some parts are already submitted? That
seems a bit weird/hard to implement for some cases (also think about object
store implementations outside this repository).
IMHO the clearer API would be to extend `put_multipart` with the enum (i.e.
another parameter) or add an "options" struct to that method (similar how other
methods have options structs nowadays). This change is breaking anyway, so we
might also use this opportunity to do this proper.
##########
src/aws/mod.rs:
##########
@@ -534,16 +541,24 @@ impl MultipartUpload for S3MultiPartUpload {
})
}
+ fn set_put_mode(&mut self, mode: PutMode) -> Result<()> {
Review Comment:
See my comment below on that method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
