Client Side Caching

[altanozlu]

Hello everyone,
Client Side Caching is almost ready, just needs more command coverage and more testing.
I changed creating of new clients with ConnectionConfig because I think it will be ugly to repeating with_cache functions everywhere.
I had to change pin-project-lite to pin-project since lite pin-project-lite doesn't support cfg feature in it.

[nihohit]

This is a very large PR, containing multiple layers of changes, which should be split into separate PRs in order to allow for better review.

1. the changes to the Client API aren't backwards compatible - at the very least we should deprecate the old functions with a message pointing towards the right function, instead of just removing the old functions. this should be a leading PR, and the Config object should use a builder pattern, similar to what's used in the cluster client's config.
2. can you explain the changes to pin-project-lite? where in this PR can we see that requirement? In any case, this, too, should be a separate PR.
3. I haven't gone over the whole code yet. If there are other sub-changes, they should be spun out to separate PRs too.

[altanozlu]

This is a very large PR, containing multiple layers of changes, which should be split into separate PRs in order to allow for better review.

1. the changes to the `Client` API aren't backwards compatible - at the very least we should deprecate the old functions with a message pointing towards the right function, instead of just removing the old functions. this should be a leading PR, and the Config object should use a builder pattern, similar to what's used in the cluster client's config.

2. can you explain the changes to `pin-project-lite`? where in this PR can we see that requirement? In any case, this, too, should be a separate PR.

3. I haven't gone over the whole code yet. If there are other sub-changes, they should be spun out to separate PRs too.

• I will fix the changes to the Client API and make it backwards compatible, didn't mean to remove them right away. By being like Cluster client's config you mean ConnectionConfigBuilder.build should create the connection ? Or it shouldn't contain ConnectionConfig.
• I wrote in PR's description but let me explain more, pin-project-lite's macro doesn't support other macros in it self and I need to put CacheManager behind a feature flag in PipelineSink. Instead of having two seperate crates for same thing I have switched to pin-project everywhere.

Converting to draft for now as I have intended

[nihohit]

related to using a config object - #1147 (comment)
not sure why I thought that the config needs the builder pattern. It's nice, but not required.

[nihohit]

By being like Cluster client's config you mean ConnectionConfigBuilder.build should create the connection ? Or it shouldn't contain ConnectionConfig.

Sorry, I don't know why I didn't answer this beforehand. I think that if you'll take Client::get_connection_manager_with_backoff_and_timeouts, and the other various Client::get_connection_manager* functions, they should become something like

client.get_tokio_connection_manager_with_config(ConnectionManagerConfig {
  response_timeout: Duration::from_millis(10), 
  Default::default()
})

or

client.get_tokio_connection_manager_with_config( 
  ConnectionManagerBuilder::new()
    .set_response_timeout(Duration::from_millis(10))
    .build() 
})

completely removing Client, or just leaving it for backwards compatibility, is also valid, but might be too much of a breaking change. We'll need to update all of the documentation, for starters.

ConnectionManagerBuilder::new(address)
   .set_authentication(auth)
   .set_response_timeout(Duration::from_millis(10))
   .connect().await

[altanozlu]

By being like Cluster client's config you mean ConnectionConfigBuilder.build should create the connection ? Or it shouldn't contain ConnectionConfig.

Sorry, I don't know why I didn't answer this beforehand. I think that if you'll take Client::get_connection_manager_with_backoff_and_timeouts, and the other various Client::get_connection_manager* functions, they should become something like

client.get_tokio_connection_manager_with_config(ConnectionManagerConfig {
  response_timeout: Duration::from_millis(10), 
  Default::default()
})

or

client.get_tokio_connection_manager_with_config( 
  ConnectionManagerBuilder::new()
    .set_response_timeout(Duration::from_millis(10))
    .build() 
})

completely removing Client, or just leaving it for backwards compatibility, is also valid, but might be too much of a breaking change. We'll need to update all of the documentation, for starters.

ConnectionManagerBuilder::new(address)
   .set_authentication(auth)
   .set_response_timeout(Duration::from_millis(10))
   .connect().await

I also like the first approach, I'll wait for main branch to get this approach and rebase over it then.