Skip to content
This repository was archived by the owner on Jul 27, 2022. It is now read-only.
This repository was archived by the owner on Jul 27, 2022. It is now read-only.

Ensure all ObjectStores have publicly available client #10

Closed

Description

@wjones127

ObjectStores might want to be extended with additional traits that use the underlying client. For example, in delta-rs we would want to extend the client to implement a version of rename that failed if the destination already exists (see delta-io/delta-rs#610).

AmazonS3 does expose a method .client():

object_store_rs/src/aws.rs

Lines 533 to 535 in 5f488c5

impl AmazonS3 {
/// Get a client according to the current connection limit.
async fn client(&self) -> SemaphoreClient {

MicrosoftAzure only has a private field container_client:

pub struct MicrosoftAzure {
container_client: Arc<ContainerClient>,

GoogleCloudStorage only has a private field client:

object_store_rs/src/gcp.rs

Lines 103 to 104 in 5f488c5

pub struct GoogleCloudStorage {
client: Client,

They all will return a different type, but maybe we should implement for each a client() method like the S3 has?

Activity

  1. tustvold commented on May 20, 2022

    @tustvold
    Contributor

    I'm a bit wary of exposing the clients used internally as I suspect we may want to change these down the line. Both Rusoto and cloud_storage are not official crates, with somewhat patchy support.

    Perhaps we could add copy support directly to the trait, it seems like a reasonable addition to me?

  2. wjones127 commented on May 20, 2022

    @wjones127
    ContributorAuthor

    Perhaps we could add copy support directly to the trait, it seems like a reasonable addition to me?

    As in, copy the client rather than returning a reference / shared pointer?

    I'm a bit wary of exposing the clients used internally as I suspect we may want to change these down the line.

    Yeah, I agree. The alternative would be crates should bring their object store extensions into this crate as optional features. So delta-rs would bring in the rename_noreplace() method as an optional trait. In the S3 case, that might mean bringing over dynamodb lock, which may be more than you wish to accept (or maybe not!).

    Or even a final alternative would be that in delta-rs we wrap the ObjectStore implementors instead of using them directly.

  3. tustvold commented on May 20, 2022

    @tustvold
    Contributor

    As in, copy the client rather than returning a reference / shared pointer?

    I meant adding CopyObject support to ObjectStore itself. A rename is just a copy followed by a delete.

    The alternative would be crates should bring their object store extensions into this crate as optional features

    I think an optional feature that adds CreateIfNotExists and CopyIfNotExists operations would be really cool. It's a real pain that S3 doesn't natively support it, and if we can somehow hide that from users that would be awesome.

  4. roeap commented on May 20, 2022

    @roeap
    Contributor

    In case of Adls Gen2 the *IfNotExist methods are natively supported. In delta-rs we also have an implementation for local file systems, which only on windows needs some extra care - I think... for blob and S3 afaik some external service is needed.

    I'd be happy to contribute adls gen2 to this crate 🙂.

  5. wjones127 commented on May 21, 2022

    @wjones127
    ContributorAuthor

    I meant adding CopyObject support to ObjectStore itself. A rename is just a copy followed by a delete.

    Sounds good. I've started implementing that in #14. I will close #11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions