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

[release/9.0] Add more specific messages when pending model changes are detected #35221

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

AndriySvyryd
Copy link
Member

@AndriySvyryd AndriySvyryd commented Nov 27, 2024

Fixes #35133
Addresses comments in #34431, #35080 and #35181

Description

EF offers a way of updating the database schema to match the current model via incremental migrations. The common scenario is to add a new migration, then update the database to the latest migration. But it is easy to forget adding a new migration after the model was changed. So, in 9.0 we added a runtime warning that would throw by default when updating the database if there are any changes detected in the model that aren't reflected in the latest migration.

However, there are several reasons why the migrations could be out-of-date and the exception message was not helpful for some of the users. This PR adds more specific messages and makes the warning to be logged instead of throwing for some cases:

  1. There are no migrations at all. This is common when the database is updated through other means. The exception will not be thrown.
  2. There are some migrations, but the model snapshot is missing. This is common for migrations added manually. The exception will not be thrown.
  3. The model wasn't modified by the developer, but it's built in a non-deterministic way causing EF to detect it as modified. This is common when new DateTime() or Guid.NewGuid() are used in seed data. The exception will still be thrown.
  4. For other cases the exception will still be thrown.

We will also add documentation explaining the above.

Customer impact

After the upgrade some applications that were working correctly started crashing on startup. This was by-design, but the required action from the developer wasn't clear. The workaround is to ignore the warning using options.

How found

Multiple customer reports on 9.

Regression

Yes, from 8.

Testing

Tests added.

Risk

Low.

@AndriySvyryd AndriySvyryd requested a review from a team November 27, 2024 04:43
@AndriySvyryd AndriySvyryd force-pushed the Issue35133 branch 2 times, most recently from 6dfea6d to d361fec Compare November 27, 2024 04:54
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

Looks great - just suggesting a bit more info in the non-deterministic message.

src/EFCore.Relational/Properties/RelationalStrings.resx Outdated Show resolved Hide resolved
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.

2 participants