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

Breaking change in OnConfiguring() not documented #4274

Closed
mhkolk opened this issue Mar 1, 2023 · 3 comments · Fixed by #4297
Closed

Breaking change in OnConfiguring() not documented #4274

mhkolk opened this issue Mar 1, 2023 · 3 comments · Fixed by #4297

Comments

@mhkolk
Copy link

mhkolk commented Mar 1, 2023

After rerunning the scaffold-dbcontext in EF Core 7 I stumbled upon an issue that does not seem to be documented in breaking change or anywhere else for that matter.

Include your code

This is the code that the scaffold-dbcontext creates in EF Core 6

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            if (!optionsBuilder.IsConfigured)
            {
#warning To protect potentially sensitive information in your connection string, you should move it out of source code. You can avoid scaffolding the connection string by using the Name= syntax to read it from configuration - see https://go.microsoft.com/fwlink/?linkid=2131148. For more guidance on storing connection strings, see http://go.microsoft.com/fwlink/?LinkId=723263.
                optionsBuilder.UseNpgsql("Host=localhost:5432;Database=SetupPlatform;Username=kolektor;Password=setup");
            }
        }

As you can see there is an if present which only runs the code with the generated connnection string if optionsBuilder instance is not present.

EF Core 7 however does this and this makes our solution to no longer work (database not found), since the connection string provided at startup through configuration is not applied due to the missing if statement:

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
#warning To protect potentially sensitive information in your connection string, you should move it out of source code. You can avoid scaffolding the connection string by using the Name= syntax to read it from configuration - see https://go.microsoft.com/fwlink/?linkid=2131148. For more guidance on storing connection strings, see http://go.microsoft.com/fwlink/?LinkId=723263.
        => optionsBuilder.UseNpgsql("Host=localhost:5432;Database=SetupPlatform;Username=kolektor;Password=setup");

This change only brings trouble and lost time to devs.

Apparently this is not the only such change in EF Core 7
https://stackoverflow.com/questions/75422460/can-i-force-entity-framework-core-7-to-not-make-icollections-read-only/75422780#75422780

Someone apparently gives little thought to this sort of things resulting in wasted development time.

Include provider and version information

EF Core version: 7.0.3
Database provider: either SqlServer or Npgsql
Target framework: .NET 7.0

@ajcvickers
Copy link
Member

Duplicate of dotnet/efcore#30168.

Note for triage: consider documenting this as a breaking change.

@mhkolk
Copy link
Author

mhkolk commented Mar 1, 2023

This is by design.

LOL. Some smart design indeed.

@mhkolk mhkolk closed this as completed Mar 1, 2023
@ajcvickers ajcvickers reopened this Mar 1, 2023
@ajcvickers ajcvickers transferred this issue from dotnet/efcore Mar 2, 2023
@ajcvickers ajcvickers self-assigned this Mar 2, 2023
@ajcvickers ajcvickers added this to the 8.0.0 milestone Mar 2, 2023
ajcvickers added a commit that referenced this issue Mar 26, 2023
ajcvickers added a commit that referenced this issue Mar 26, 2023
ajcvickers added a commit that referenced this issue Mar 26, 2023
@Tinsulate
Copy link

Tinsulate commented Aug 16, 2023

Is just lost two development days hunting for this issue, as this caused some database configurations to fail very silently. Suffice to say I wasn't too happy.

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

Successfully merging a pull request may close this issue.

3 participants