feat(provisioning-device, prov-amqp, prov-mqtt, prov-https): Add support for timespan timeouts to provisioning device client

[timtay-microsoft]

As discussed in #2036, AMQP provisioning device clients have no way to configure the timeout given to the AMQP library for operations like opening links. This adds overloads to the existing provisioning device client's registerAsync methods that allow for users to configure this timespan timeout.

Old API surface for provisioning device client (which didn't use optional params for some reason?)

public Task<DeviceRegistrationResult> RegisterAsync();

public Task<DeviceRegistrationResult> RegisterAsync(ProvisioningRegistrationAdditionalData data);

public Task<DeviceRegistrationResult> RegisterAsync(CancellationToken cancellationToken);

public Task<DeviceRegistrationResult> RegisterAsync(ProvisioningRegistrationAdditionalData data, CancellationToken cancellationToken);

Proposed new API surface

public Task<DeviceRegistrationResult> RegisterAsync(CancellationToken cancellationToken = default);

public Task<DeviceRegistrationResult> RegisterAsync(ProvisioningRegistrationAdditionalData data, CancellationToken cancellationToken = default);

public Task<DeviceRegistrationResult> RegisterAsync(TimeSpan timeout);

public Task<DeviceRegistrationResult> RegisterAsync(ProvisioningRegistrationAdditionalData data, TimeSpan timeout);

[timtay-microsoft]

I'm not sure why we didn't use optional parameters here before, but I'm assuming

public Task<DeviceRegistrationResult> RegisterAsync(CancellationToken cancellationToken = default)

is equivalent to

public Task<DeviceRegistrationResult> RegisterAsync();
public Task<DeviceRegistrationResult> RegisterAsync(CancellationToken cancellationToken);

When CancellationToken.None is passed as the token from RegisterAsync() to RegisterAsync(CancellationToken)

[drwill-ms]

It is, but not binary compatible. Could be seen as a breaking change. I'd be okay with it, though. Simply requires recompile.

[timtay-microsoft]

A bit of a tangential change, but this is a polling interval not a pooling interval

[abhipsaMisra]

amqp just loves pooling everything

[timtay-microsoft]

s_timeoutConstant was the hardcoded 1 minute timespan that we passed into the AMQP library to use in the place of a cancellation token. This PR allows users to control what that timespan is configured to instead of this hardcoded value

[timtay-microsoft]

Since HTTP and MQTT do support using cancellation tokens all the way through, we can just create a cancellation token out of the timespan here.

[timtay-microsoft]

I was tempted to make the TimeSpan an optional parameter here like the cancellation token is, but we can't have two overloads with the same first parameter and different optional parameters

public Task<DeviceRegistrationResult> RegisterAsync(ProvisioningRegistrationAdditionalData data, TimeSpan timeout = default);
public Task<DeviceRegistrationResult> RegisterAsync(ProvisioningRegistrationAdditionalData data, CancellationToken cancellationToken = default);

public void myMethod()
{
    provisioningDeviceClient.RegisterAsync(data); // Which of the two above options would the compiler choose?
}

[drwill-ms]

Hmm

Could always add TimeSpan as another optional parameter to the one that has a CancellationToken. Would that be less weird?

[timtay-microsoft]

I could go either way on this suggestion. If we ever get cancellation token support for our AMQP library, then we can just deprecate all the APIs that take TimeSpans, but if we start mixing TimeSpans and Cancellation tokens then it gets messy.

[abhipsaMisra]

I'd like to keep the timespan and cancellation token APIs separate. Since both are ways to signal an API that it should stop the in-progress operation gracefully, I don't think we should mix them together.

[timtay-microsoft]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[abhipsaMisra]

Unit tests/ E2E tests?

[timtay-microsoft]

Unit tests/ E2E tests?

Working on a few E2E tests now, yeah

[timtay-microsoft]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[timtay-microsoft]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).