From 8dcadfdd9e203156fc4341f845524a4796401a8b Mon Sep 17 00:00:00 2001 From: Kyle Barron Date: Mon, 21 Oct 2024 00:17:23 -0400 Subject: [PATCH 1/2] Consolidate copy/rename if not exists --- .../python/object_store_rs/_copy.pyi | 39 ++++-------- .../python/object_store_rs/_rename.pyi | 32 +++------- object-store-rs/src/copy.rs | 57 ++++++----------- object-store-rs/src/rename.rs | 61 ++++++------------- 4 files changed, 61 insertions(+), 128 deletions(-) diff --git a/object-store-rs/python/object_store_rs/_copy.pyi b/object-store-rs/python/object_store_rs/_copy.pyi index 3e911cd3..6eba9693 100644 --- a/object-store-rs/python/object_store_rs/_copy.pyi +++ b/object-store-rs/python/object_store_rs/_copy.pyi @@ -1,41 +1,26 @@ from .store import ObjectStore -def copy(store: ObjectStore, from_: str, to: str) -> None: +def copy(store: ObjectStore, from_: str, to: str, *, overwrite: bool = True) -> None: """Copy an object from one path to another in the same object store. - If there exists an object at the destination, it will be overwritten. - Args: store: The ObjectStore instance to use. from_: Source path to: Destination path - """ - -async def copy_async(store: ObjectStore, from_: str, to: str) -> None: - """Call `copy` asynchronously. - - Refer to the documentation for [copy][object_store_rs.copy]. - """ - -def copy_if_not_exists(store: ObjectStore, from_: str, to: str) -> None: - """ - Copy an object from one path to another, only if destination is empty. - - Will return an error if the destination already has an object. - Performs an atomic operation if the underlying object storage supports it. - If atomic operations are not supported by the underlying object storage (like S3) - it will return an error. + Keyword Args: + overwrite: If `True`, if there exists an object at the destination, it will + be overwritten. Performs an atomic operation if the underlying object + storage supports it. If atomic operations are not supported by the + underlying object storage (like S3) it will return an error. - Args: - store: The ObjectStore instance to use. - from_: Source path - to: Destination path + If `False`, will return an error if the destination already has an object. """ -async def copy_if_not_exists_async(store: ObjectStore, from_: str, to: str) -> None: - """Call `copy_if_not_exists` asynchronously. +async def copy_async( + store: ObjectStore, from_: str, to: str, *, overwrite: bool = True +) -> None: + """Call `copy` asynchronously. - Refer to the documentation for - [copy_if_not_exists][object_store_rs.copy_if_not_exists]. + Refer to the documentation for [copy][object_store_rs.copy]. """ diff --git a/object-store-rs/python/object_store_rs/_rename.pyi b/object-store-rs/python/object_store_rs/_rename.pyi index 0c30c490..2f5d2529 100644 --- a/object-store-rs/python/object_store_rs/_rename.pyi +++ b/object-store-rs/python/object_store_rs/_rename.pyi @@ -1,41 +1,27 @@ from .store import ObjectStore -def rename(store: ObjectStore, from_: str, to: str) -> None: +def rename(store: ObjectStore, from_: str, to: str, *, overwrite: bool = True) -> None: """ Move an object from one path to another in the same object store. By default, this is implemented as a copy and then delete source. It may not check when deleting source that it was the same object that was originally copied. - If there exists an object at the destination, it will be overwritten. - Args: store: The ObjectStore instance to use. from_: Source path to: Destination path + + Keyword Args: + overwrite: If `True`, if there exists an object at the destination, it will be + overwritten. If `False`, will return an error if the destination already has + an object. """ -async def rename_async(store: ObjectStore, from_: str, to: str) -> None: +async def rename_async( + store: ObjectStore, from_: str, to: str, *, overwrite: bool = True +) -> None: """Call `rename` asynchronously. Refer to the documentation for [rename][object_store_rs.rename]. """ - -def rename_if_not_exists(store: ObjectStore, from_: str, to: str) -> None: - """ - Move an object from one path to another in the same object store. - - Will return an error if the destination already has an object. - - Args: - store: The ObjectStore instance to use. - from_: Source path - to: Destination path - """ - -async def rename_if_not_exists_async(store: ObjectStore, from_: str, to: str) -> None: - """Call `rename_if_not_exists` asynchronously. - - Refer to the documentation for - [rename_if_not_exists][object_store_rs.rename_if_not_exists]. - """ diff --git a/object-store-rs/src/copy.rs b/object-store-rs/src/copy.rs index 6976c89f..5c8e0774 100644 --- a/object-store-rs/src/copy.rs +++ b/object-store-rs/src/copy.rs @@ -6,63 +6,46 @@ use pyo3_object_store::PyObjectStore; use crate::runtime::get_runtime; #[pyfunction] +#[pyo3(signature = (store, from_, to, *, overwrite = true))] pub(crate) fn copy( py: Python, store: PyObjectStore, from_: String, to: String, + overwrite: bool, ) -> PyObjectStoreResult<()> { let runtime = get_runtime(py)?; + let from_ = from_.into(); + let to = to.into(); py.allow_threads(|| { - runtime.block_on(store.as_ref().copy(&from_.into(), &to.into()))?; + let fut = if overwrite { + store.as_ref().copy(&from_, &to) + } else { + store.as_ref().copy_if_not_exists(&from_, &to) + }; + runtime.block_on(fut)?; Ok::<_, PyObjectStoreError>(()) }) } #[pyfunction] +#[pyo3(signature = (store, from_, to, *, overwrite = true))] pub(crate) fn copy_async( py: Python, store: PyObjectStore, from_: String, to: String, + overwrite: bool, ) -> PyResult> { + let from_ = from_.into(); + let to = to.into(); pyo3_async_runtimes::tokio::future_into_py(py, async move { - store - .as_ref() - .copy(&from_.into(), &to.into()) - .await - .map_err(PyObjectStoreError::ObjectStoreError)?; - Ok(()) - }) -} - -#[pyfunction] -pub(crate) fn copy_if_not_exists( - py: Python, - store: PyObjectStore, - from_: String, - to: String, -) -> PyObjectStoreResult<()> { - let runtime = get_runtime(py)?; - py.allow_threads(|| { - runtime.block_on(store.as_ref().copy_if_not_exists(&from_.into(), &to.into()))?; - Ok::<_, PyObjectStoreError>(()) - }) -} - -#[pyfunction] -pub(crate) fn copy_if_not_exists_async( - py: Python, - store: PyObjectStore, - from_: String, - to: String, -) -> PyResult> { - pyo3_async_runtimes::tokio::future_into_py(py, async move { - store - .as_ref() - .copy_if_not_exists(&from_.into(), &to.into()) - .await - .map_err(PyObjectStoreError::ObjectStoreError)?; + let fut = if overwrite { + store.as_ref().copy(&from_, &to) + } else { + store.as_ref().copy_if_not_exists(&from_, &to) + }; + fut.await.map_err(PyObjectStoreError::ObjectStoreError)?; Ok(()) }) } diff --git a/object-store-rs/src/rename.rs b/object-store-rs/src/rename.rs index 08a4440d..c71348fc 100644 --- a/object-store-rs/src/rename.rs +++ b/object-store-rs/src/rename.rs @@ -6,67 +6,46 @@ use pyo3_object_store::PyObjectStore; use crate::runtime::get_runtime; #[pyfunction] +#[pyo3(signature = (store, from_, to, *, overwrite = true))] pub(crate) fn rename( py: Python, store: PyObjectStore, from_: String, to: String, + overwrite: bool, ) -> PyObjectStoreResult<()> { let runtime = get_runtime(py)?; + let from_ = from_.into(); + let to = to.into(); py.allow_threads(|| { - runtime.block_on(store.as_ref().rename(&from_.into(), &to.into()))?; + let fut = if overwrite { + store.as_ref().rename(&from_, &to) + } else { + store.as_ref().rename_if_not_exists(&from_, &to) + }; + runtime.block_on(fut)?; Ok::<_, PyObjectStoreError>(()) }) } #[pyfunction] +#[pyo3(signature = (store, from_, to, *, overwrite = true))] pub(crate) fn rename_async( py: Python, store: PyObjectStore, from_: String, to: String, + overwrite: bool, ) -> PyResult> { + let from_ = from_.into(); + let to = to.into(); pyo3_async_runtimes::tokio::future_into_py(py, async move { - store - .as_ref() - .rename(&from_.into(), &to.into()) - .await - .map_err(PyObjectStoreError::ObjectStoreError)?; - Ok(()) - }) -} - -#[pyfunction] -pub(crate) fn rename_if_not_exists( - py: Python, - store: PyObjectStore, - from_: String, - to: String, -) -> PyObjectStoreResult<()> { - let runtime = get_runtime(py)?; - py.allow_threads(|| { - runtime.block_on( - store - .as_ref() - .rename_if_not_exists(&from_.into(), &to.into()), - )?; - Ok::<_, PyObjectStoreError>(()) - }) -} - -#[pyfunction] -pub(crate) fn rename_if_not_exists_async( - py: Python, - store: PyObjectStore, - from_: String, - to: String, -) -> PyResult> { - pyo3_async_runtimes::tokio::future_into_py(py, async move { - store - .as_ref() - .rename_if_not_exists(&from_.into(), &to.into()) - .await - .map_err(PyObjectStoreError::ObjectStoreError)?; + let fut = if overwrite { + store.as_ref().rename(&from_, &to) + } else { + store.as_ref().rename_if_not_exists(&from_, &to) + }; + fut.await.map_err(PyObjectStoreError::ObjectStoreError)?; Ok(()) }) } From 935588af73ca05320c107d0145f643682c1469d2 Mon Sep 17 00:00:00 2001 From: Kyle Barron Date: Mon, 21 Oct 2024 00:18:44 -0400 Subject: [PATCH 2/2] fix lib --- object-store-rs/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/object-store-rs/src/lib.rs b/object-store-rs/src/lib.rs index 33c942b6..c7802e11 100644 --- a/object-store-rs/src/lib.rs +++ b/object-store-rs/src/lib.rs @@ -25,8 +25,6 @@ fn _object_store_rs(py: Python, m: &Bound) -> PyResult<()> { pyo3_object_store::register_store_module(py, m, "object_store_rs")?; m.add_wrapped(wrap_pyfunction!(copy::copy_async))?; - m.add_wrapped(wrap_pyfunction!(copy::copy_if_not_exists_async))?; - m.add_wrapped(wrap_pyfunction!(copy::copy_if_not_exists))?; m.add_wrapped(wrap_pyfunction!(copy::copy))?; m.add_wrapped(wrap_pyfunction!(delete::delete_async))?; m.add_wrapped(wrap_pyfunction!(delete::delete))?; @@ -45,8 +43,6 @@ fn _object_store_rs(py: Python, m: &Bound) -> PyResult<()> { m.add_wrapped(wrap_pyfunction!(put::put_file_async))?; m.add_wrapped(wrap_pyfunction!(put::put_file))?; m.add_wrapped(wrap_pyfunction!(rename::rename_async))?; - m.add_wrapped(wrap_pyfunction!(rename::rename_if_not_exists_async))?; - m.add_wrapped(wrap_pyfunction!(rename::rename_if_not_exists))?; m.add_wrapped(wrap_pyfunction!(rename::rename))?; m.add_wrapped(wrap_pyfunction!(signer::sign_url_async))?; m.add_wrapped(wrap_pyfunction!(signer::sign_url))?;