Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify cosmos client construction #886

Merged
merged 3 commits into from
Jul 14, 2022

Conversation

yoshuawuyts
Copy link
Contributor

This PR builds on #884 to slim down the instantiation of the cosmos client. It does away with CosmosOptions and adds more lenient bounds on the constructor. This PR is best reviewed on a per-commit basis. Thanks!

Example

// before
let client = CosmosClient::new(
    account.clone(),
    authorization_token,
    CosmosOptions::default(),
);
// after
let client = CosmosClient::new(&account, authorization_token);

Details

The CosmosOptions struct is a thin wrapper around the azure_core::ClientOptions struct. It doesn't expose any custom fields, and forwards all calls directly to the internal ClientOptions struct.

CosmosOptions has two constructors: the empty new method /Default implementation, and the new_with_transaction_name method. The latter which is only used inside the CosmosClient::new_with_transaction method, so removing it hasn't removed any existing functionality.

We know that we want to expose the ability to add custom pipeline plugins (middleware) to the clients. But this can be done by exposing builder methods on the client, and we don't need an Options struct to achieve this. On the contrary: the work in this PR is intended as prep work to enable the implementation of a builder structure.

So to summarize: no existing functionality is lost, and this sets us up to implement pipeline extensions using builder methods on the client.

@yoshuawuyts yoshuawuyts force-pushed the simplify-client-construction branch from 608648e to 2863511 Compare July 14, 2022 08:54
@rylev rylev marked this pull request as ready for review July 14, 2022 08:58
@yoshuawuyts yoshuawuyts merged commit 9a77fe5 into Azure:main Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants