Background and information around the 9.0 PendingModelChangesWarning error

[roji]

Everyone, we know some of you are (understandably) frustrated by the new PendingModelChangesWarning error that gets thrown when upgrading to EF 9. I wanted to share some information on the new error, why we did it and what you should do about it.

The main motivation behind this change was something that happens often: you make some change to your model (e.g. add a property), and then you forget to actually add a migration to apply it to the database. You run MigrateAsync() (or similar) and can't figure out why your database hasn't been updated; the new error clearly tells you that your model contains changes with respect to the last migration snapshot. In other words, the point here is to help you avoid accidental error.

Now, there are some other cases where this error is thrown, even if you haven't done any changes; code that seemed to run just fine in EF 8 suddenly starts throwing in EF 9. A lot of people have assumed that this is a problem in EF, but in reality the error usually still indicates some problem in your code that was there before, but was not flagged. That's a good thing! Even here, the point is to help you clean up the problem and avoid problems. Of course, you always have the option of simply suppressing the warning (see below), so you should never be blocked for upgrading to EF 9; it's fine to do this temporarily, and then maybe come back and deal with it later, after upgrading, once you have more time to focus on it.

Here's an example: some people want to seed their database with some random Guids, so they put code such as the following in their model configuration:

modelBuilder.Entity<Blog>().HasData(new Blog { Id = Guid.NewGuid(), ... });

This looks simple enough, but it's actually quite problematic. For each migration, EF stores a snapshot of your model for that migration. When you create a new migration, EF compares your current model (represented by your code) with the latest snapshot, and the new migration represents the difference between these two. In 9.0, when you call MigrateAsync, EF does the exact same thing, and if any changes are detected, throws the new PendingModelChangesWarning error.

Now, when you generated your last migration, Guid.NewGuid() got evaluated at that point, and an actual random Guid gets integrated into the previous context snapshot; you can open up the XXXContextModelSnapshot.cs in your migrations directory, and see that Guid. Now, when you call MigrateAsync, Guid.NewGuid() gets evaluated again, but because the random Guid it generates will differ from the one stored in the model snapshot, EF detects this as a pending model change (as if you forgot to add a migration!).

The solution to this is very simple: avoid using dynamic (or non-deterministic) APIs such as Guid.NewGuid(), and embed specific GUIDs instead:

modelBuilder.Entity<Blog>().HasData(new Blog { Id = Guid.Parse("9e4f49fe-0786-44c6-9061-53d2aa84fab3"), ... });

Once you do this, generate a new migration once, and this whole thing will be behind you.

Why is this important? Previous versions of EF may have not thrown PendingModelChangesWarning for this, but they also didn't work well around this situation. For example, every time you added a migration, EF would see a model change in the Guid seed data, and generate a useless migration to change that value in the database - that's a bad state of affairs that the new error helps detect and fix. So while the new error is frustrating, it's there to help you get your code in better shape.

There are some other cases where this error will be thrown; we've recently published a breaking change note listing these, and explaining what to do. Also, since the error has been causing some confusion, we recently merged an improvement that will be released in 9.0.1: EF will now recognize some of the issues causing the "pending model changes" error, and throw more specific errors to let you know exactly what the problem is. For example, the case above with Guid.NewGuid() will now be detected, and we'll tell you that you're using dynamic values in HasData.

Hopefully the above clarifies things; we recommend looking into it and trying to fix the root cause - this way you end up with a better, more stable application. But if you choose to, you can suppress the warning, and revert back to the same situation as in previous versions of EF:

options.ConfigureWarnings(w => w.Ignore(RelationalEventId.PendingModelChangesWarning))

[pdevito3]

I appreciate the clarity and get the desire here to try and help. This error probably will help people new to EF too, but at the very least something here seems off in something like my use case.

As far as i knew, I had no dynamic or non-deterministic APIs. All of my migrations are straight EF generated with zero migration modifications and no HasData usages or other seeds. The closest I have to this is is my BaseEntity (entity, not in a migration) having a default for the id prop of NewGuid():

public abstract class BaseEntity
{
    [Key]
    public Guid Id { get; private set; } = Guid.NewGuid();
    //...
}

From what i saw in the original ticket this is somehow getting picked up, but I know of no no other way to do this in code and not doing so would be bad dx as id's would show as empty guids until save. I don't see this showing up in my snapshot either.

Is there an alternative you guys recommend? Or are we just supposed to live with a bad dx here or add explicit code that overrides this error?

[roji]

@pdevito3 yeah, initializing your property to a Guid property on your CLR type in this way is effectively the same as including it inside HasData(): every time the model runs, the seeding data that's included in the model has a different Guid (since your HasData has a new MyEntity() in it, which causes a new Guid to get generated).

To fix this, you can:

• Make your Id setter public, and set it explicitly to some value within HasData()
• If you don't want to make the setter public, you can switch to using an anonymous type inside HasData(). For example, instead of having HasData(new MyEntity { Foo = "..." }), you can have HasData(new { Id = Guid.Parse("..."), Foo = "..." }). This would restrict to change to your HasData() configuration, without impacting what your CLR type looks like.

[ajcvickers]

@roji I think @pdevito3 is saying they have no calls to HasData:

with zero migration modifications and no HasData usages or other seeds.

[rudiv]

I'm glad the breaking change note is there now (there needs to be one for temporal migrations too #35108 as it's impossible to upgrade if you use them).

Agree with @pdevito3 that this is great for new developers to know what's going on and why their code is throwing errors at runtime, but I don't understand why this isn't handled better and with correct terminology.

the new PendingModelChangesWarning error

Is it actually meant to be a warning or an error? If it truly was a warning, that yellow log printing to the console on build would be extremely helpful so that we know we need to do something. It could also throw as an error at the time when it would actually cause the runtime exception rather than being an ambiguous "column not found" (or similar) error. The current behaviour of having it exploding vs not having it at all is worse.

Typically on larger changes to models, for example either early stages where the data structure needs to change to support what is being built out, or later stages where an entire model is being changed, we want to be actively on the system whilst simultaneously updating code for the models until we get the result that we desire. To do this now, we'd have to disable warnings, rebuild, incrementally make our changes, and then migrate and put the warning back in.

#35221 seems to be a step in the right direction where certain types of the same warning won't throw an exception which is awesome! But the use case above as far as I can see is still going to throw an exception by default? I may be being stupid as I've only had a cursory look at the PR.

Surely it would be better to have multiple types of "warnings" so that we can suppress them individually rather than all on and off?

[pdevito3]

@roji I think @pdevito3 is saying they have no calls to HasData:

with zero migration modifications and no HasData usages or other seeds.

Correct. This is in my base entity model that all my dbsets inherit from. This default makes it so my entities get an actual id value when created in code instead of waiting for save changes to do it for me. Without a default guid like this, the id is always empty until save. I had this in a project once and it was very much bad dx.

[roji]

@ajcvickers @pdevito3 sorry, I missed that important sentence :) In that case @pdevito3 it would be good to have a repro, to make sure this isn't some case we're not aware of... If you can put that together, can you please open a new issue containing that repro?