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

Expose connection string in a migration. Closes #240 #414

Merged
merged 3 commits into from May 24, 2013

Conversation

daniellee
Copy link
Contributor

The connection string that is passed into the runner is now exposed as a property in the MigrationBase class.

I need a bit of feedback on this. This works as long as the connection string is set on the RunnerContext. This happens with the three runners (console, Nant and MSBuild) but for those who go directly against the MigrationRunner class then it has to be set on the RunnerContext that is then passed into the ctor of MigrationRunner.

The other way to do it would be to fetch the connection string from the Processor class. This is always set otherwise it is impossible to execute against a database. The problem is that the code for this is a bit more complicated. The JetProcessor has an oledb connection whereas all the other processors have an IDBConnection and I'd have to cast the processor to GenericProcessorBase or to JetProcessor. So it's all a bit messy but it would be guaranteed to be the real connection string.

What do you think?

The connection string that is passed into the runner is now exposed as
property in the MigrationBase class.
If no connection string is set in the RunnerContext and
is instead loaded by ConfigStringManager then set the
Connection property so that it is passed onto the
MigrationRunner.
@ghost ghost assigned tommarien May 23, 2013
@tommarien
Copy link
Contributor

@daniellee i'll merge this in this evening, our processor could just expose the connectionstring, so IProcessor contract could expose readonly connectionstring property. But for now i think this pull is fine, we can fine tune later ;)

@daniellee
Copy link
Contributor Author

Then I might as well change it first. I started off doing that way and then changed my mind.

@tommarien
Copy link
Contributor

@daniellee mmm, i agree but you where talking about casting which is not needed because processor knows which connectiontype it uses :P ( i thought you where talking about violating liskov substition principle)

@daniellee
Copy link
Contributor Author

True, I'll fix it without casting by adding the connection string property to the processor interface instead. Thanks for the suggestion.

@tommarien
Copy link
Contributor

@daniellee thanks for the fix ;)

Expose conn string in MigrationBase which is fetched from the Processor
class when MigrationRunner is executed and passed down to MigrationBase.
@daniellee
Copy link
Contributor Author

My first solution didn't feel right. This feels much better. What do you think of this @tommarien?

@tommarien
Copy link
Contributor

@daniellee i'll pull it in but will change something important, returning Connectionstring property after opening connection will not retain password entered in connectionstring, or for sqlserver you should specify Persist Security info. I will change the code so that the connectionstring will always return string passed in constructor.

See http://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlconnection.connectionstring.aspx for more details

@daniellee
Copy link
Contributor Author

Thanks, didn't think of that.

@daniellee daniellee deleted the issue240 branch May 24, 2013 19:50
@tommarien
Copy link
Contributor

@daniellee Like i allready said thanks for the pull and for doing most of the work ;)

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

Successfully merging this pull request may close these issues.

None yet

2 participants