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

Add ConfigureDataSource() to NpgsqlDbContextOptionsBuilder #3277

Merged
merged 1 commit into from
Sep 14, 2024

Conversation

roji
Copy link
Member

@roji roji commented Sep 14, 2024

Closes #2542
Closes #2704

/cc @NinoFloris

@roji roji changed the title Add ConfigureDataSource() to NpgsqlDbCOntextOptionsBuilder Add ConfigureDataSource() to NpgsqlDbContextOptionsBuilder Sep 14, 2024
Copy link
Member

@NinoFloris NinoFloris left a comment

Choose a reason for hiding this comment

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

Nice :)

}

[Fact]
public void Data_source_config_with_same_connection_string_and_different_lambda()
Copy link
Member

Choose a reason for hiding this comment

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

Very helpful test!

Copy link
Member Author

Choose a reason for hiding this comment

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

Right 🤣 More like documentation to remind future self what's going on...

@@ -18,6 +18,13 @@ public NpgsqlDbContextOptionsBuilder(DbContextOptionsBuilder optionsBuilder)
{
}

/// <summary>
Copy link
Member

Choose a reason for hiding this comment

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

Should we want a remark on the semantics here? Something in the direction of: "Changes made by ConfigureDataSource are untracked. For OnConfiguring identical configuration through other methods on NpgsqlDbContextOptionsBuilder will cause EF Core to resolve the same DbDataSource, even when ConfigureDataSource changes in future calls of OnConfiguring. When referencing outside state in ConfigureDataSource keep it static or ensure it also causes changes to other NpgsqlDbContextOptionsBuilder configuration."

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

Twaked the message a bit, but otherwise same idea.

@roji
Copy link
Member Author

roji commented Sep 14, 2024

@NinoFloris also pushed a check to throw when both an (external) NpgsqlDataSource is passed to UseNpgsql() and the data source config lambda is defined.

@roji roji enabled auto-merge (squash) September 14, 2024 23:30
@roji roji merged commit 30cebf0 into npgsql:main Sep 14, 2024
15 of 16 checks passed
@roji roji deleted the ConfigureDataSource branch September 14, 2024 23:34
@altso
Copy link

altso commented Sep 19, 2024

@roji will this be available in 8.x.x?

@roji
Copy link
Member Author

roji commented Sep 20, 2024

No, this is a significant, complex change, and as the milestone on the issues linked the PR show, it's for 9.0.

@altso
Copy link

altso commented Sep 20, 2024

Got it. Thanks for the explanation.

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