MikaelGRA / InfluxDB.Client

InfluxDB Client for .NET
MIT License
102 stars 22 forks source link

Cancellation of operation #58

Closed marcinjahn closed 4 years ago

marcinjahn commented 4 years ago

The API does not expose any way to cancel an ongoing operation. Probably it would be a good idea to add CancellationToken to each async call. Another thing would be to add an option of specifying a timeout, that would be useful.

Currently, I have an extension method like this:

        public static async Task TimeoutAfter(this Task task, int timeoutInSeconds)
        {
            if (task == await Task.WhenAny(task, Task.Delay(TimeSpan.FromSeconds(timeoutInSeconds))))
                await task;
            else
                throw new TimeoutException($"The operation took longer than {timeoutInSeconds} seconds.");
        }

It would be better if the library had cancellation support.

marcinjahn commented 4 years ago

@MikaelGRA Is it something that you'd like to see in the client? If so, I will create a PR with CancellationToken support for each endpoint. I'd add it as an optional parameter (CancellationToken cancellationToken = default).

marcinjahn commented 4 years ago

@MikaelGRA I noticed your thumbs-up for my first post, so I took it as a "yes" :) Have a look: https://github.com/MikaelGRA/InfluxDB.Client/pull/65/files

MikaelGRA commented 4 years ago

Hey.

Sorry about that. Been very busy lately so not been paying much attention. Remember looking at the issue back then.

Wanted to do it but was a bit jaded about the effort required to do it in a way that did not break any existing APIs binary compability (making overloads of everything instead of adding parameters).

But I feel like perhaps this way is better. I will find some time to merge this request. Sorry for no noticing earlier.

marcinjahn commented 4 years ago

@MikaelGRA No issue at all. I actually thought that adding an optional parameter is going to be “safe” in terms of compatibility. In what cases might it be problematic?

MikaelGRA commented 4 years ago

In terms of recompilation (of a client library to this) there is no issue because the parameter has a default value. But the binary compability is broken because there no longer is a method with the same parameters so if you were to simply update the DLL for this library in a running system it would throw a MissingMethodException when the method whose signatures were changed were to be called.

Something that is probably not ever going to happen. ^_^

MikaelGRA commented 4 years ago

Thanks for the PR. I've pulled it in pretty much as is and included it in version 4.0.4. 👍