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

Support passing up CancellationTokens to ToPagedListAsync(). #25

Open
VictorioBerra opened this issue Oct 25, 2018 · 4 comments
Open

Support passing up CancellationTokens to ToPagedListAsync(). #25

VictorioBerra opened this issue Oct 25, 2018 · 4 comments

Comments

@VictorioBerra
Copy link
Contributor

VictorioBerra commented Oct 25, 2018

ASPNET Core automatically wires up a CancellationToken in your controller actions, so it would be really helpful if we could add ToListAsync() to the paging stuff.

MVC will automatically bind any CancellationToken parameters in an action method to the HttpContext.RequestAborted token, using the CancellationTokenModelBinder. This model binder is registered automatically when you call services.AddMvc() (or services.AddMvcCore()) in Startup.ConfigureServices().
via https://andrewlock.net/using-cancellationtokens-in-asp-net-core-mvc-controllers/

We would put it here and pass it all the way up to the calling code https://github.com/AlexanderKrutov/DataTables.Queryable/blob/master/DataTables.Queryable/PagedList.cs#L78 and this way we could avoid doing Task.Factory stuff like here https://github.com/AlexanderKrutov/DataTables.Queryable/blob/master/DataTables.Queryable/QueryableExtensions.cs#L78

@VictorioBerra VictorioBerra changed the title async support so we can use CancellationTokens. Support passing up CancellationTokens to ToPagedListAsync(). Oct 25, 2018
@VictorioBerra
Copy link
Contributor Author

I have been attempting this, turns out I have bumped into an issue with the ToListAsync() and CountAsync() in EFCore and keeping net40 as target framework.

Honestly, I think we should drop support for net40 and only support netstandard. This is exactly the kind of problem netstandard is trying to solve.

@AlexanderKrutov
Copy link
Owner

Hi @VictorioBerra,
thank you so much for the pull request!
Unfortunately I can not approve this without additional changes. The are some reasons why:

  1. We should not discard the net40 support. The library is used in some projects targeted to this platform, so we must keep it.
  2. Your changes require that the library depends on Microsoft.EntityFrameworkCore. Initially the lib is designed to work with any data source, not only EF. For example, it can be in-memory data set, and we should not limit users in this case.
  3. CancellationToken support is definetely needful for ASP.Net. But I'm not sure how it will work with different web servers (like Nancy which I'm use with the DataTables.Queryable). Additional investigation is needed here. I think it should be more universal way to integrate CancellationToken support without linking with EF and ASP.Net.

I appreciate your contribution and will keep the pull as a starting point for further investigation.

@VictorioBerra
Copy link
Contributor Author

Thanks a lot for looking at this I appreciate it. For now I think I can solve my immediate issue by doing my own paging so I'm the one calling ToListAsync and passing down the CancellationToken. It would be nice to see this implemented some day though.

-Tory

@VictorioBerra
Copy link
Contributor Author

You could keep the ASPNET Core web port though. I may PR you some tests when I can get around to it.

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

No branches or pull requests

2 participants