aerospike-client-rust icon indicating copy to clipboard operation
aerospike-client-rust copied to clipboard

derive `Clone` for `Client`

Open databasedav opened this issue 1 year ago • 2 comments

any reason we wouldn't want to do this? the internal Cluster is already wrapped in an arc, also just anecdotal but all the other client structs i've worked with (nats and sqlx) allow this cloning so the user doesn't need to do the Arc wrapping in their application

databasedav avatar May 17 '23 02:05 databasedav

I have nothing against it other than caution for the future. How does it differ ergonomically if we do that? Can you share some code?

khaf avatar May 26 '23 09:05 khaf

the ergonomic benefit is simply not needing to wrap the client struct in an Arc in generic-specifying contexts, the examples i have are from using axum e.g. when declaring request extractors

#[async_trait]
impl<S> axum::extract::FromRequestParts<S> for Data
where
    Arc<aerospike::Client>: axum::extract::FromRef<S>,
    S: Send + Sync,
{
    type Rejection = axum::response::Response;

    async fn from_request_parts(parts: &mut http::request::Parts, state: &S) -> Result<Self, Self::Rejection> { ... }
}

~in fact, because Client doesn't impl Clone, axum requires another manual impl for this to work~

impl FromRef<StateWrapper> for Arc<aerospike::Client> {
    fn from_ref(state: &StateWrapper) -> Self {
        state.0.aerospike_client.clone()
    }
}

~e.g. the nats and sqlx clients don't require this extra impl to use them in request extractors because they do impl Clone~

edit: was wrong about the second thing

databasedav avatar Jun 17 '23 21:06 databasedav