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

DbMigrator should avoid connecting to master and should use existing connection if possible #522

Closed
divega opened this issue Apr 16, 2018 · 3 comments · Fixed by #1153
Closed

Comments

@divega
Copy link
Contributor

divega commented Apr 16, 2018

The current implementation causes issues, e.g. when using ADD authentication with Azure SQL DB.

From @ajcvickers's email:

... it seems that there is an issue with the database existence check that is done by the DbMigrator—it is attempting to connect to the master database, which fails. We had updated other existence checks to not require this, but it looks like DbMigrator was either missed or there is some reason it couldn’t be done here...

Also, there is apparently a code path there that may leave a connection open.

@ajcvickers ajcvickers self-assigned this Jul 6, 2018
@ajcvickers ajcvickers added this to the 6.3.0 milestone Jul 6, 2018
@GraemeSMiller
Copy link

Is there any work around for this? Everything is marked private or internal etc. I hoped to just override update in a certain part of my code and avoid call to EnsureDatabaseExists but without taking huge amount of EF code this doesn't seem possible

@gvasko
Copy link

gvasko commented Feb 12, 2019

Hi,
we have been experimenting with a workaround which seems working so far, but not heavily tested.

  1. we created an AzureSqlClientFactory class that extends DbProviderFactory
  • it just wraps the SqlClientFactory, everything is delegated to it
  • except the CreateConnection, in which we add the token
  1. created an AzureDbResolver class that implements IDbProviderFactoryResolver
  • returns AzureSqlClientFactory instance if the connection is SqlConnection
  • returns EntityProviderFactory.Instance if the connection is EntityConnection
  1. in the ctor of the DbConfiguration subclass
  • SetProviderFactoryResolver(new AzureDbResolver());
  • SetMigrationSqlGenerator("My.AzureSqlClient", () => new SqlServerMigrationSqlGenerator());
  1. app config
	<system.data>
		<DbProviderFactories>
			<add name="My Azure Provider Wrapper" invariant="My.AzureSqlClient" support="FF" description="Wrapper for Azure"
				type="My.AzureSqlClientFactory, MyAssembly"/>
		</DbProviderFactories>
	</system.data>
	<entityFramework>
		<defaultConnectionFactory type="My.AzureSqlClientFactory, MyAssembly" />
		<providers>
			<provider invariantName="My.AzureSqlClient" type="System.Data.Entity.SqlServer.SqlProviderServices, EntityFramework.SqlServer"/>
		</providers>
	</entityFramework>

(We use a separated executable for database migration with its own app config, so this workaround doesn't affect the web app.)

Do you think this can help?

@ajcvickers ajcvickers changed the title Upgrade database existence check in DbMigrator to avoid connecting to master DbMigrator should avoid connecting to master and should use existing connection if possible Apr 3, 2019
@ajcvickers
Copy link
Contributor

Additional note: this is problematic when an existing connection has an access token associated. We cant then clone the connection.

ajcvickers added a commit that referenced this issue Aug 16, 2019
This prevents migrations having to attempt to create connections from connection strings.

Fixes #522
ajcvickers added a commit that referenced this issue Aug 17, 2019
This prevents migrations having to attempt to create connections from connection strings.

Fixes #522
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.

5 participants