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.

Sign up for GitHub

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

Jump to bottom

Improve Task.Run()/CancellationToken code #4577

Merged
merged 3 commits into from
Jul 18, 2024
Merged

Improve Task.Run()/CancellationToken code #4577

merged 3 commits into from
Jul 18, 2024

Conversation

viceroypenguin
Copy link
Contributor

Part of #4475

@viceroypenguin
Copy link
Contributor Author

/azp run test-all

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@linq2dbot linq2dbot mentioned this pull request Jul 17, 2024
Closed
@linq2dbot
Copy link

Test baselines changed by this PR. Don't forget to merge/close baselines PR after this pr merged/closed.

MaceWindu
MaceWindu requested changes Jul 18, 2024
View reviewed changes
Copy link
Contributor

@MaceWindu MaceWindu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see code still assume Provider could be not IQueryProviderAsync. Remove it here or you plan to submit another PR?

Source/LinqToDB/AsyncExtensions.cs Show resolved Hide resolved
@viceroypenguin
Copy link
Contributor Author

I see code still assume Provider could be not IQueryProviderAsync. Remove it here or you plan to submit another PR?

I think you're incorrect in your assumption here. For example, if someone calls Enumerable.Range(1, 10).AsQueryable().DeleteAsync() (stupid I know, but syntactically and semantically legal with regards to the language), it's possible to hit the Task.Run().

@MaceWindu
Copy link
Contributor

If there is no IQueryProviderAsync, then it is not linq2db query provider and calling it with Linq2db API will result in error from provider, in this case System.Linq.EnumerableExecutor:

System.InvalidOperationException: 'There is no method 'Delete' on type 'LinqToDB.LinqExtensions' that matches the specified arguments'

Instead of trying to execute broken code we can throw exception with descriptive message that linq2db API used over non-linq2db query.

PS: if there are valid cases I'm not aware of - we could preserve it

@viceroypenguin
Copy link
Contributor Author

@MaceWindu fair enough - let's do a sep PR for that.

@viceroypenguin viceroypenguin merged commit 0aa0753 into master Jul 18, 2024
4 checks passed
@viceroypenguin viceroypenguin deleted the features/task-start branch July 18, 2024 12:21
@MaceWindu MaceWindu added this to the 6.0.0-preview.2 milestone Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaceWindu MaceWindu MaceWindu approved these changes

@igor-tkachev igor-tkachev Awaiting requested review from igor-tkachev igor-tkachev is a code owner

@sdanyliv sdanyliv Awaiting requested review from sdanyliv sdanyliv is a code owner

@jods4 jods4 Awaiting requested review from jods4 jods4 is a code owner

@Shane32 Shane32 Awaiting requested review from Shane32 Shane32 is a code owner

Assignees
No one assigned
Labels
None yet
Milestone
6.0.0-preview.2
Development

Successfully merging this pull request may close these issues.
3 participants
@viceroypenguin @linq2dbot @MaceWindu