fsprojects / FSharp.Data.SqlClient

A set of F# Type Providers for statically typed access to MS SQL database
http://fsprojects.github.io/FSharp.Data.SqlClient/
Other
205 stars 71 forks source link

Include Task-based execution on SqlCommandProvider #371

Open VTJDailey opened 4 years ago

VTJDailey commented 4 years ago

It would be nice to have the option of using Task rather than Async when combining workflows with other asynchronous behaviors. Pretty much any third party library in the .NET ecosystem is going to be using Task and it would be handy to be able to jump right into that without converting from Async.

I am proposing to add the method 'TaskExecute' on the command generated from SqlCommandProvider. This method which would act in parallel to the currently existing 'AsyncExecute'.

I'm happy to work at implementing this if the package maintainers think it's a good idea and would be likely to merge it in.

smoothdeveloper commented 4 years ago

@VTJDailey thanks for the suggestion (and proposing for implementation!), I've used a bit the async methods but I'm not acquainted with the underlying ADO.NET implementation which seems to expose Begin/End methods as well as methods returning Task<>.

Is there specific gains to have Task versus the current way of using Async.StartAsTask on the existing async returning methods?

I've not had problem myself using Async.StartAsTask when I wanted tasks out of the async values.

I'm not oposed to get task methods but they do feel redundant and I'd personally like a good case / more insight for why Task is better and Async.StartAsTask doesn't cut it.

Looking for more feedback and perspective from the users and the other maintainers of the library.

If this is a pressing need, maybe you could expand on current pain you have?

Couple more general questions:

VTJDailey commented 4 years ago

@smoothdeveloper thanks for the update!

Certainly using Async.StartAsTask is an option. However, the .net standard version of that function is already a Task<> wrapped in an Async<>. It would be equivalent to:

Async.StartAsTask(Async.AwaitTask(cmd.ExecuteNonQueryAsync(/* this returns a Task*/)))

So I'm worried about the performance of wrapping the Async<> back in another Task<>. I'm not familiar enough with the internals of StartAsTask and AwaitTask. Will layering Task over Async over Task perform the same as just a single Task? Or is it consuming an unnecessary thread or resources?

As for the Begin/End methods on the older NET40 version of ADO.NET code, Tasks have a factory method to be generated from the Begin/End methods in the same way as Async, so that's not a problem.

I agree that it is a bit redundant to have AsyncExecute and TaskExecute, but that's just a natural extension of the fact that it's a bit redundant to have Async<> and Task<>, even if they're not exactly the same. And for better or worse, Task is the .NET standard for asynchronous programming. Everything in the base class library uses Task, almost every third party NuGet package is going to use it. Only packages narrowly targeting F# users are going to ever expose Async<> at their endpoints.

I guess the short version of the request is that I would never use the Async<> returned from a command, I would convert it to a Task literally every single time.

A second potential reason would be that it would make it a bit easier to 'sell' this package to be used within a small F# data access assembly as part of a larger project that is written entirely in C#. Including Task versions is a way of 'playing nicely' with the larger ecosystem we live in.

I'm not aware of an upcoming task builder in future versions of FSharp.Core, at least not for sure. There is this library which would be helpful to reference in implementing this issue: https://github.com/rspeele/TaskBuilder.fs