KEMBAR78
Add template methods around DbConnection inside RelationalConnection by joaopgrassi · Pull Request #22849 · dotnet/efcore · GitHub
Skip to content

Conversation

@joaopgrassi
Copy link
Member

Resolves #21327

@joaopgrassi joaopgrassi marked this pull request as draft September 30, 2020 08:20
@joaopgrassi
Copy link
Member Author

joaopgrassi commented Sep 30, 2020

There's a test failing (RelationalApiConsistencyTest) due to the newly added template method for DbConnection.CloseAsync()... Not sure how to proceed. The method is protected, so I can't add to the exclusion list (AsyncMethodExceptions). If I make it public then it's not consistent with the rest of the template methods.. and just adding the cancellation token to make the tests pass seems hacky (since it's not used by DbConnection.CloseAsync). Any ideas?

}

DbConnection.EnlistTransaction(transaction);
ConnectionEnlistTransaction(transaction);
Copy link
Contributor

Choose a reason for hiding this comment

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

@dotnet/efteam Any thoughts on naming these template methods before merging? (We will review in API review later anyway.)

@ajcvickers
Copy link
Contributor

@joaopgrassi You should be able to add the method to AsyncMethodExceptions even though it is protected. Something like:

typeof(RelationalConnection).GetMethod("CreateDbConnection", BindingFlags.Instance | BindingFlags.NonPublic);

@joaopgrassi
Copy link
Member Author

joaopgrassi commented Sep 30, 2020

typeof(RelationalConnection).GetMethod("CreateDbConnection", BindingFlags.Instance | BindingFlags.NonPublic);

Right 😅. Was so deep into the errors forgot the obvious, lol.

I did notice though that some methods return a ValueTask but they were not being caught by this consistency check. Looking more I saw that's because inside Async_methods_should_have_overload_with_cancellation_token_and_end_with_async_suffix it just looks for Task. Not sure if it's something to change as well.. probably not now since it will produce errors everywhere.

@joaopgrassi joaopgrassi marked this pull request as ready for review October 1, 2020 07:33
@ajcvickers ajcvickers merged commit b715a35 into dotnet:main Oct 5, 2020
@ajcvickers
Copy link
Contributor

@joaopgrassi Thanks for the contribution!

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.

Consider adding template methods for DbConnection calls

2 participants