search

dotnet / ef6

This is the codebase for Entity Framework 6 (previously maintained at https://entityframework.codeplex.com). Entity Framework Core is maintained at https://github.com/dotnet/efcore.
https://docs.microsoft.com/ef/ef6
MIT License
1.41k stars 538 forks source link

EF 6.5 release checklist #2237

Closed AndriySvyryd closed 3 weeks ago

AndriySvyryd commented 3 months ago
ErikEJ commented 3 months ago

Whoa!!

Are you planning any preview releases? 🙏

AndriySvyryd commented 3 months ago

Yes, but we haven't decided on how many there will be.

AndriySvyryd commented 2 months ago

We've just released a preview version of Microsoft.EntityFramework.SqlServer 6.5.0. Please try it and let us know if you run into any issues.

ErikEJ commented 2 months ago

@AndriySvyryd Amazing news, Andriy !

Bartleby2718 commented 2 months ago

Great news for sure, but @AndriySvyryd what's the reason behind the release? Is it for Microsoft.Data.SqlClient support?

ErikEJ commented 2 months ago

@Bartleby2718 Yes, and some small Dependency and tooling updates

chandlerkent commented 2 months ago

@AndriySvyryd just wanted to let you know we've migrated from ErikEJ.EntityFramework.SqlServer to Microsoft.EntityFramework.SqlServer@6.5.0-preview2-24180-01 and have been running it in our test environments without issue.

Is there an ETA at getting a released version?

Thanks!

AndriySvyryd commented 2 months ago

@chandlerkent We plan to release the final version next month. It will likely be identical to 6.5.0-preview2

jons-bakerhill commented 1 month ago

Not sure if it's the switch to Microsoft.Data.SqlClient or the switch to the new provider but in my local test migration I'm seeing queries timeout because of transactions that didn't timeout previously. I'm honestly not sure how the code as written worked in the past so something is probably behaving more correctly now but I'm not sure what.

Roughly speaking (it's legacy code, not how we do it now) we have 2 DbContext instances. One is used for the data modification and the other is used for pre/post object retrieval for auditing changes. When the modification context is used in a transaction and the query to get the modified object is in the transaction block (but not part of it because it's a different connection) for comparison times out because the transaction is still open.

As I typed out the above maybe previously the connection got reused more or cleaned up less and somehow the "audit" context snuck into the transaction? A pretty wild guess though, seems unlikely.

ErikEJ commented 1 month ago

@jons-bakerhill You post seems quite off topic here, but have you tried using 5.2.0, which has this fix? https://github.com/dotnet/SqlClient/pull/2301

jons-bakerhill commented 1 month ago

Somewhere along the way I was thinking of this as a "please try this and provide feedback" issue and lost the "checklist" part of it in my head.

It's not the issue you linked though. The exceptions I'm getting are definitely code misbehaving and trying to query data that is changed in a transaction where the connection doing the query isn't part of that transaction. I'm a bit puzzled how it ever worked and thought I'd ask/note that it seems like either the new provider or the new version of EF itself seems to be "more correct" now. It could be something else entirely.

ErikEJ commented 1 month ago

@jons-bakerhill No problem, I prefer to get these reports now. Maybe it is a "bad behaviour" that has been fixed now, is that what you are saying?

If you can provide a simple console app repro I can take a quick look, otherwise

jons-bakerhill commented 1 month ago

Maybe it is a "bad behaviour" that has been fixed now, is that what you are saying?

Yes.

It will likely be a few business days (at best) for me to have time to set up a console app reproduction but I'll try to find some time to do so.

jons-bakerhill commented 1 month ago

It looks like our command interceptors weren't getting added anymore (they set the transaction to READ UNCOMMITTED, so the transaction lock is ignored).

We relied on having a class derived from DbConfiguration public class RegisterInterceptors : DbConfiguration in the same assembly to register them and after adding/switching to using the DbConfigurationType(typeof(MicrosoftSqlDbConfiguration))] attribute on the DbContext it no longer is called. Maybe something for the readme on the NuGet package for Microsoft.EntityFramework.SqlServer https://www.nuget.org/packages/Microsoft.EntityFramework.SqlServer/#readme-body-tab

I've updated our code to add the interceptors in the API service startup code and removed the derived class. Things appear to be working fine again.

ErikEJ commented 1 month ago

@jons-bakerhill some sample code with your fix would be very helpful if you think this is a candidate for readme inclusion

ErikEJ commented 1 month ago

So did you have code like this?

public class FE6CodeConfig : DbConfiguration { public FE6CodeConfig() { this.AddInterceptor(new EFCommandInterceptor()); } }

And did you try to change to:

public class FE6CodeConfig : MicrosoftSqlDbConfiguration { public FE6CodeConfig() { this.AddInterceptor(new EFCommandInterceptor()); } }

jons-bakerhill commented 1 month ago

I believe adding [DbConfigurationType(typeof(MicrosoftSqlDbConfiguration))] to the DbContext broke the automatic registration of the interceptors via deriving from DbConfiguration

[DbConfigurationType(typeof(MicrosoftSqlDbConfiguration))]
public class DerivedDBContext : DbContext

So I replaced 

using System.Data.Entity;

namespace Project.Interceptors
{
    public class RegisterInterceptors : DbConfiguration
    {
        public RegisterInterceptors()
        {
            AddInterceptor(new SetSecuritySessionContext());
            AddInterceptor(new IsUserActiveInterceptor());
            AddInterceptor(new SetSecuritySessionSysDbContext());
        }
    }
}

with DbInterception.Add calls in Startup.cs

[assembly: OwinStartup(typeof(OrgNamespace.Startup))]
namespace ApiNamespace
{
    public partial class Startup
    {
        public static void Configuration(IAppBuilder app)
        {
            // Removed boiler plate code

            DbInterception.Add(new SetSecuritySessionContext());
            DbInterception.Add(new IsUserActiveInterceptor());
            DbInterception.Add(new SetSecuritySessionSysDbContext());

        }
    }
}
ErikEJ commented 1 month ago

Thanks, but did you try deriving from MicrosoftSqlDbConfiguration ?

jons-bakerhill commented 1 month ago

Thanks, but did you try deriving from MicrosoftSqlDbConfiguration ?

I just tried again with the code below after commenting out my startup additions and it was never called.

using System.Data.Entity.SqlServer;

namespace OrgNamespace.Interceptors
{
    public class RegisterInterceptors : MicrosoftSqlDbConfiguration
    {
        public RegisterInterceptors()
        {
            AddInterceptor(new Interceptor1());
            AddInterceptor(new Interceptor2());
            AddInterceptor(new Interceptor3());
        }
    }
}

image

ErikEJ commented 1 month ago

Since you already have a derived DbConfiguration class, I believe you should do like this (as documented, but maybe the wording could be inproved)

Or you can add the following lines to your existing DbConfiguration class:

SetProviderFactory(MicrosoftSqlProviderServices.ProviderInvariantName, Microsoft.Data.SqlClient.SqlClientFactory.Instance); SetProviderServices(MicrosoftSqlProviderServices.ProviderInvariantName, MicrosoftSqlProviderServices.Instance); // Optional SetExecutionStrategy(MicrosoftSqlProviderServices.ProviderInvariantName, () => new MicrosoftSqlAzureExecutionStrategy());

jons-bakerhill commented 1 month ago

I believe you should do like this

Did something get lost from the first half of your previous comment?

Or you can add the following lines to your existing DbConfiguration class:

I'm not sure how adding lines to the derived RegisterInterceptors class would help since it's not getting called in the first place.

It took a bit to click but I think what I need to do if I want to use the attribute to point the context to the new Microsoft.Data provider is to point directly to my derived class instead of the base class. That is [DbConfigurationType(typeof(RegisterInterceptors))] instead of [DbConfigurationType(typeof(MicrosoftSqlDbConfiguration))] which seems to be working in my testing so far.

ErikEJ commented 1 month ago

I'm not sure how adding lines to the derived RegisterInterceptors class would help since it's not getting called in the first place.

You should not use the attribute in this case. Notice the Or conditions in the readme.

ErikEJ commented 1 month ago

MicrosoftSqlDbConfiguration is not a base class, it is just a convenince helpers.

jons-bakerhill commented 1 month ago

MicrosoftSqlDbConfiguration is not a base class, it is just a convenince helpers.

Inheriting from it (public class RegisterInterceptors : MicrosoftSqlDbConfiguration) instead of DbConfiguration and using the derived class in the DbConfigurationType attribute seems to work fine. How is that not using it as a base class?

My suggestion (based on what I think is finally a mostly complete understanding) would be to add a note along the lines of "If you are using a customized DbConfiguration by deriving from it in the same assembly as your DbContext you need to call DbConfiguration.SetConfiguration or apply the DbConfigurationType attribute. Make sure to use your custom class in the call or attribute and make sure it is based on MicrosoftSqlDbConfiguration"

I have a couple options at this point, I just want to keep the configuration out of web.config to ease the migration to .Net 6+. It's just a matter of deciding which one will work best in our solution.

Thank you for working through this with me, it's much appreciated!

ErikEJ commented 1 month ago

I think this clarification is enough, but I will test it:

Or add the following lines to your existing derived DbConfiguration class:

jons-bakerhill commented 1 month ago

I think this clarification is enough, but I will test it:

Or add the following lines to your existing derived DbConfiguration class:

Oh... your prior comment https://github.com/dotnet/ef6/issues/2237#issuecomment-2120791994 makes more sense now. Without quotes or a link it wasn't clear to me that was a full quote from the readme page. I'm a little slow on the uptake today it appears, must be because it's a Monday :D

3nth commented 1 month ago

Not sure if this is the readme to note this, but if you are using EF6.5 on .NET Core with Microsoft.SqlServer.Types (DbGeography) then you are limited to deploying to windows and you must package with a windows runtime identifier and --self-contained true or the DbGeography won't load.

For Azure Web Apps at least. Not sure if it's an issue for an IIS deployment.

ErikEJ commented 1 month ago

@3nth Please share a simple console app that demonstrates this issue, and I will have a closer look.

3nth commented 1 month ago

@ErikEJ This is looking like a peculiarity with Azure App Services.

Runtime identifier does appear to be required due to the windows only libraries in Microsoft.SqlServer.Types.

With regards to --self-contained I've found that the net6 MVC app deployed to Azure App Services does require --self-contained true. With false get this error when DbGeography.Distance() is used.

Message: Specified type is not registered on the target server. Microsoft.SqlServer.Types.SqlGeography, Microsoft.SqlServer.Types, Version=16.0.0.0, Culture=neutral, PublicKeyToken=89845dcd8080cc91.
Microsoft.Data.SqlClient.SqlCommand+<>c.<ExecuteDbDataReaderAsync>b__211_0(Task`1 result):8

A peculiarity is we also have a net6 console app that gets run in the background via WebJobs and it crashes at start unless --self-contained false is used. This one is hosted on Azure App Services as well, but the web app itself is .NET Framework MVC (last one to migrate). I don't think this is related to anything here, the console app itself runs just fine on the host via remote shell, so something with WebJobs hosting more likely.

None of this has been reproduceable locally. Cannot make sense of it, but we have a working deployment now.

3nth commented 1 month ago

With regards to --self-contained I've found that the net6 MVC app deployed to Azure App Services does require --self-contained true. With false get this error when DbGeography.Distance() is used.

This seems to have been some oddity with one of the slots. deleting that slot and re-creating it seems to have cleared the issue. Nothing to do with --self-contained, it was simply failing every other deployment.

ErikEJ commented 1 month ago

@AndriySvyryd would you like a PR for a docs page with a readme?

Are we waiting for 5.1.6?

AndriySvyryd commented 4 weeks ago

@AndriySvyryd would you like a PR for a docs page with a readme?

Yes, that would be great.

Are we waiting for 5.1.6?

No, I don't think it will be released this month

AndriySvyryd commented 3 weeks ago

https://www.nuget.org/packages/EntityFramework/ https://www.nuget.org/packages/EntityFramework.SqlServerCompact/ https://www.nuget.org/packages/Microsoft.EntityFramework.SqlServer/

me-manikanta commented 3 weeks ago

I see that there are two version of 6.5(6.5.0 and 6.5.1), but I can't find the change log. Can you help with what the changes are in these versions?

ErikEJ commented 3 weeks ago

@me-manikanta https://github.com/dotnet/ef6/releases/tag/v6.5.1