search

JordanMarr / SqlHydra

SqlHydra is a suite of NuGet packages for working with databases in F# including code generation tools and query expressions.
MIT License
212 stars 20 forks source link

Fix cancellation support #60

Closed jwosty closed 11 months ago

jwosty commented 11 months ago

Related to #19

jwosty commented 11 months ago

While working on this I noticed how QueryContext marshalling Task to Async to Task again - what is the reason for this? I see comments saying "Must wrap in async to prevent EndExecuteNonQuery ex in NET6_0_OR_GREATER" - what exception is this referring to, and how does it prevent keeping it all in task-land?

Example: https://github.com/JordanMarr/SqlHydra/blob/d03ef91afbe21a1fd57e924561e311ba999f2c74/src/SqlHydra.Query/QueryContext.fs#L99

JordanMarr commented 11 months ago

While working on this I noticed how QueryContext marshalling Task to Async to Task again - what is the reason for this? I see comments saying "Must wrap in async to prevent EndExecuteNonQuery ex in NET6_0_OR_GREATER" - what exception is this referring to, and how does it prevent keeping it all in task-land?

Example:

https://github.com/JordanMarr/SqlHydra/blob/d03ef91afbe21a1fd57e924561e311ba999f2c74/src/SqlHydra.Query/QueryContext.fs#L99

I'm a little hazy on that, but here is some background:

This project started before the task CE was introduced. I planned to swap to the native task at some point, but I also didn't want to inadvertently break anything, so decided that put it off a while longer.

I believe that error affected a Xamarin app I was writing, but I don't recall the details. It's basically a "here be dragons" warning.

Do you want to take a stab at upgrading it to the new task?

jwosty commented 11 months ago

@JordanMarr great, just converted it over. Also took the liberty to async-ify some more bits (each part is in separate commits so if you don't like one of them it should be easy to partially revert). Feel free to do your own manual tests to ensure I didn't break anyone else's scenarios.

JordanMarr commented 11 months ago

These additions are really nice! Thank you for the hard work. I will test and then probably release a beta version first so that I can run it against a few of my projects.

JordanMarr commented 11 months ago

Should these be converted to tasks as well?

https://github.com/JordanMarr/SqlHydra/blob/f5ddd5533ff1896d5bcf5ae74e0919fa0e9bf7d1/src/SqlHydra.Query/SelectBuilders.fs#L355C9-L355C35

EDIT: I migrated them to tasks.

JordanMarr commented 11 months ago

Query v2.0.3-beta is posted. Perhaps we need to also add netstandard2.1 as a build target so that the BeginTransactionAsync is available?