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 cancellationToken to async methods, add QueryAsync command method… #1625

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlexeyVasiliev
Copy link

… with Array of types in the recordset

@NickCraver
Copy link
Member

Unfortunately - these are all breaking changes. It's something we'll address in the next major version of Dapper, but aren't aiming to do in the current APIs due to the breaking nature (adding optional params would break all libraries compiled on top of Dapper - it's not binary compatible).

I'll leave this open for any questions, but aim to cleanup and close out in a few days if all is understood here.

@Herostwist
Copy link

Unfortunately - these are all breaking changes. It's something we'll address in the next major version of Dapper, but aren't aiming to do in the current APIs due to the breaking nature (adding optional params would break all libraries compiled on top of Dapper - it's not binary compatible).

Would adding new overloads with non optional cancellationToken papramater still be considered a breaking change?

@NickCraver
Copy link
Member

Would adding new overloads with non optional cancellationToken papramater still be considered a breaking change?

It would not, but it would create a lot of extra methods to maintain. Given the plan to change the overall interface of how we layer in vNext, I think we'd want to add them once...and take this breaking change since people would need to compile against it anyway.

@DSchougaard
Copy link

Would it instead be possible to grant directly access to the currently private methods executing this, that accepts CommandDefinitions (which does have CancellationTokens), in the mean time?

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

Successfully merging this pull request may close these issues.

None yet

4 participants