aerospike / aerospike-client-rust

Rust client for the Aerospike database
https://www.aerospike.com/
Other
82 stars 26 forks source link

derive `Clone` for `Client` #133

Open databasedav opened 1 year ago

databasedav commented 1 year ago

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

khaf commented 1 year ago

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?

databasedav commented 1 year ago

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