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

Issue with Issue 35 Fix #71

Closed
jonreis opened this issue Aug 9, 2012 · 6 comments
Closed

Issue with Issue 35 Fix #71

jonreis opened this issue Aug 9, 2012 · 6 comments

Comments

@jonreis
Copy link

jonreis commented Aug 9, 2012

I think the new behavior is bad. Here is why.

Before the "fix", If you wanted an one-time-script to run again, all you had to do was rename it. If you didn't want it to run, it didn't. Now there is no way of telling RH not to run a modified one-time-script. This will cause loads of problems with some of our existing installs.

I propose we go back to the old behavior where the user has control over which scripts get re-executed. Anyone else?

Here is the information from Issue 35:

"Currently, if you set "warnononetimescriptchanges" true, RH will report the altered script and continue running. It will not attempt to execute the updated script.

This patch changes that behavior so that if you set "warnononetimescriptchanges" true, RH reports script alteration, then runs the updated script and records the new script run information. I think this is far more useful, and I gather from my conversations with Rob, it was the expected behavior.

If you have an objection to me merging this in, please give a shout."

@BiggerNoise
Copy link
Member

Well, obviously, I am going to disagree with you.

In the Roundhouse view of the universe, changing a one time script is a bad thing. The original warnononetimescriptchanges demotes this bad thing to a warning that can be endlessly ignored.

If you have the ability to revert the script to its original state, then this is the correct thing to do. The warnononetimescripchange option was intended to be a temporary thing. Get an error, figure out that reverting the script was not practical, run RH one time with the option and make that the new baseline.

@jonreis
Copy link
Author

jonreis commented Aug 9, 2012

I thought you might. But, here is a scenario for you.

You have just release a very successful 1.0 version of you product. You branch your code and replace the CreateTable.sql script with a pristine version that was generated from your most recent database schema, and you blow away your 20 odd update scripts. Call it a little house keeping,

Now you want to upgrade an existing 1.0 customer to your new 2.0 product. But the latest version of RH with the forcing the running of one-time-scripts, will prevent the upgrade from taking place. And from what I can tell, there is no work-around to force this install.

In the old version of RH, this was possible because the CreateTable.sql would warn that it has run before, but not attempt to run the script. If you really wanted the one-time-script to run again, you would just have to rename it and RH would happily run it for you. So the "fix" didn't add anything, it only took some functionality away.

In general I agree with the commandment "Thou shall not make changes to one-time-scripts", however; I would argue that it should really be a best practice and not forced on the user by the tool.

I'm ok with the idea of having one-time-scripts run when changed, since we always write these to check if a specific change is necessary before making the change. In other words we always make these scripts re-runnable. The only script that does not follow this practice is the CreateTable script which creates the initial database schema. So perhaps the scripts that generate the initial database should be in a different folder and follow different set of update rules (ie only run once even if changed).

Just my 2-cents.

@BiggerNoise
Copy link
Member

If you re-baseline a schema, those scripts should go in RunAfterCreateDatabase. They really should not be placed in the up folder.

Now, I haven't had a chance to play with that feature when a database group actually does the database creation, but that's where I would focus my attention if we need to make any changes.

@jonreis
Copy link
Author

jonreis commented Aug 9, 2012

I guess I was missing the RunAfterCreateDatabase folder. I need to dig into the documentation. Sorry to waste your time.

@BiggerNoise
Copy link
Member

Not a waste at all, Roundhouse just gets better when exposed to real world use.

Please let us know what your experiences are with the RunAfterCreateDatabase.

@ferventcoder
Copy link
Member

Hey all. Sounds like things were worked out. :D

I think the original intention was for the script to run if you set it to warnononetimechanges to true, since the file has changed it should follow the same logic as the rest of what RH does. When you tell it to just go down to a warning, it will apply the script and move on.

WarnOnOneTimeChanges should not be used as an option all the time. It's meant to get your database migrated when there is something wrong.

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

No branches or pull requests

3 participants