Allow the user to configure the http client

[stevenrombauts]

This PR allows you to pass an array to configure the http client. We use this to control the timeout limit, for example.

The options array expects a http key:

 BlobServiceClient::fromConnectionString($this->connectionString, ['http' => ['timeout' => 123]]),

That way other options can be added later (like additional middleware, for example)

I wasn't immediately sure how to test for this change, and did not look into it any further. If you like to see some tests for this change and have any suggestions, please let me know :)

[pimjansen]

Ik not a fan of this implementation. What is the options array. Nobody had a clue what could in there

[stevenrombauts]

Sure, how about a ClientOptions class then instead of an array, that has an array property $http? Or do you prefer to have a class with properties for the Guzzle options as well?

[pimjansen]

Sure, how about a ClientOptions class then instead of an array, that has an array property $http? Or do you prefer to have a class with properties for the Guzzle options as well?

Well internally it uses guzzle as client however it is plugable with any psr http client.

What is the actual usecase to extend this config since the http client is not exposed and just for internal use

[stevenrombauts]

Thanks! Our use-case simply put is that we require different timeouts based on the request context.

Now, if we could effectively plug in our own instance of the http client, we can configure it as we want of course! I had another look but I see that GuzzleHttp\Client is set as the client property's type everywhere (looking at BlobServiceClient, BlobContainerClient and BlobClient) and I don't see any way to configure a different http client instance . Did I miss something obvious?

[pimjansen]

@brecht-vermeersch what is your view

[brecht-vermeersch]

I am not a fan of exposing all guzzle config. We should pick and choose the useful ones so we keep the freedom of switching the client in the future.

The options should be an object instead of an array. I would add a config object for every client. BlobClientOptions, BlobContainerClientOptions, ...

Each with a property called timeout

[stevenrombauts]

I can certainly make that change. One thing though, does it really make sense to create a separate options object as you mentioned? All these classes instantiate the same type of Client, so wouldn't a single ClientOptions class be sufficient? You can still configure BlobClient, BlobContainerClient and BlobServiceClient separately if required, using different ClientOptions objects.

[brecht-vermeersch]

Having a different options class per client gives us more flexibility in the future. We can add other client options independent of each other.

In the client option classes we can have a shared class for http options.

ex.

readonly class BlockBlobClientOptions {
  public __construct(
     public ?string $someOptionOnlyForBlockBlobs = null,
     public ?HttpClientOptions = null 
  ) {}
}

readonly class BlobContainerClientOptions {
  public __construct(
     public ?HttpClientOptions = null 
  ) {}
}

... other clients

readonly class HttpClientOptions {
  public __construct(public ?int $timeout = null) {}
}