[Discussion] How to declare ClientOptions

[heaths]

Starting with PR #288 we need to define how ClientOptions should be defined throughout our SDKs. Based on our general designs and .NET designs - more specifically for service versions - client options should be optional with good defaults.

In PR #288 I'm suggesting we implement the Default trait along with setters that allow for a pattern like:

let options = StorageClientOptions::default()
  .with_version("2021-06-15")
  .with_retry(RetryOptions::default().with_mode(RetryMode::Fixed()));

Alternatively, a service version could be passed into a new associated function like we do for .NET and Java, at least.

The underpinning concept is that nothing in ClientOptions should be required, or that should be in the associated Client class's new method (or similar).

The Azure SDK guidelines isn't typically specific on how client options are implemented, but it state that we should follow a convention of:

• Client is the client to send requests
• ClientOptions are options for the client above

[heaths]

Also, @rylev brought this up before, but it sounds like we should favor composition over derivation, such that a *ClientOptions class should contain an azure_core::ClientOptions named options. In our other Azure SDKs, we derive or implement, but perhaps that's not idiomatic here.

So logical structure would look like this using Storage as an example again:

#[derive(Clone, Debug, Default)]
struct StorageClientOptions {
  version: StorageClientServiceVersion,
  options: azure_core::ClientOptions,
}

I also recommend we derive Clone and Debug as well. If we don't want to borrow references everywhere, Clone will be useful. Based on our existing SDKs, nothing apart from the HttpClient (which we already define as Arc<dyn HttpClient anyway) is expensive anyway.

[rylev]

The code you posted above looks good. My only nit would be perhaps a better name for the options field, but I'm not sure what one would be.

#[derive(Clone, Debug, Default)]
struct StorageClientOptions {
  version: StorageClientServiceVersion,
  options: azure_core::ClientOptions,
}

Question: what would be the default StorageClientServiceVersion?

[heaths]

We always default to the latest (hardcoded) at the time of compilation. Release semvers never refer to service preview versions - only "beta" (we stopped using "preview" because that causes support lifecycle confusion with the service) semvers can do that, but I'm not sure how those work with cargo. I'll have that in the guidelines to match our other SDKs as well.