Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for tasks and breaking changes #26

Open
natalie-o-perret opened this issue Apr 25, 2021 · 7 comments
Open

Add support for tasks and breaking changes #26

natalie-o-perret opened this issue Apr 25, 2021 · 7 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@natalie-o-perret
Copy link
Member

natalie-o-perret commented Apr 25, 2021

Poor decision design of mine leading to inconsistent naming, what we should have instead:

  • blahblahTask
  • blahblahAsync
  • blahblah: this is synchronous stuff

Blocked by #25 => main feature of V2

@akhansari
Copy link

Opinionated: Since async is the first class feature and network calls should be asynchronous, I would say:

  • blahblah: 'T -> Async<'U>
  • blahblahTask: 'T -> Task<'U>
  • blahblahSync: 'T -> 'U

@natalie-o-perret
Copy link
Member Author

Opinionated: Since async is the first class feature and network calls should be asynchronous, I would say:

  • blahblah: 'T -> Async<'U>
  • blahblahTask: 'T -> Task<'U>
  • blahblahSync: 'T -> 'U

The issue is that our solution at the moment is not consistent with the construction part, building functions such as:

  • text
  • parameters
  • cancellationToken
    Just to name a few.
    These are all synchronous calls without being suffixed with Sync, which would make the whole lib even more tedious to use.

@akhansari
Copy link

akhansari commented Apr 26, 2021

IMOH, I'm not a big fan of prefixes and suffixes like I was in C#. I could not associate them to types and instead see them as alternatives to main functions.
For the sake of simplicity we can support only Async<>.
Or since we usually don't mix them, we can use shadowing to switch from one to another. Like:

  • open Vp.FSharp.Sql.Task (It could also be useful to choose the lib… Vp.FSharp.Sql.Ply)
  • open Vp.FSharp.Sql.Synchronous
  • And by default : Async<>

On the other hand, source functions are all Task<> and it's a bit ugly to do Task -> Async -> Task.

@natalie-o-perret
Copy link
Member Author

Or since we usually don't mix them, we can use shadowing to switch from one to another. Like:

Point taken.

That being said I like the idea of providing Task support. Some people (a sizeable number, which I would be really keen to get my hands on it), in the F# community use Tasks as their day-to-day async driver.

It's one thing to be opinionated about pushing Async first, and it's another to support Tasks without polluting extra layers in stack traces.

@natalie-o-perret
Copy link
Member Author

On the other hand, source functions are all Task<> and it's a bit ugly to do Task -> Async -> Task.

Absolutely, that was my point (i.e. stack traces).

The same can be said about synchronous calls => less pollution when you don't have to force Async.RunSynchronously and perf-wise it's still better than endlessly wrapping everything.

@akhansari
Copy link

Shadowing FTW (:

@natalie-o-perret
Copy link
Member Author

Shadowing FTW (:

My mileage may drastically vary on that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants