chucknorris / roundhouse

RoundhousE is a Database Migration Utility for .NET using sql files and versioning based on source control
http://projectroundhouse.org
917 stars 249 forks source link

Error changing everytime script inside one-time directory #114

Open mpareja opened 11 years ago

mpareja commented 11 years ago

This might be a bit of an oddball use case, but it didn't behave as I would have expected.

Reproduction:

  1. Add a script name my.EVERYTIME.sql to the Up directory.
  2. Run RH.
  3. Modify the contents of my.EVERYTIME.sql.
  4. Run RH.

Unexpected:

Roundhouse complains that my.EVERYTIME.sql has changed. I would expect that file to be treated just like if it were within an EVERYTIME directory.

ferventcoder commented 11 years ago

This is intended behavior. The Up folder is a special ONE TIME only folder. Curious why you would need this?

mpareja commented 11 years ago

As mentioned, the use case for this is awkward. What we're really trying to do is get the particular EVERYTIME script to run at a certain point in the deployment. If we had customizable workflows, we wouldn't have even tried this.

All that said, it didn't behave quite as I would have expected. Because of the script's filename, it is run every time - despite being in the ONE TIME directory. The existing behaviour is not consistent with the one time scripts behaviour, nor is it consistent with the any time scripts behaviour. Therefore, I think the correct behaviour should be either one of the following:

  1. The script is run every time and RH does NOT abort on script change; or,
  2. The script is only run once and RH aborts on script change.

Given we've defined a certain behaviour for files containing EVERYTIME in them, I think it should run as per 1.

k4gdw commented 11 years ago

The up folder is a special folder in which every script is considered a one-time script that should never be edited after it has been run on a particular database regardless of any tags such as ENV or EVERYTIME embedded in the file name. The way I understand it, If files in the up folder will run EVERYTIME if so marked, Ifbut if they've been changed the will complain. If you want it editable, put it in one of the anytime folders where any file that has edited files, plus those marked EVERYTIME, get run without complaint from rh.

mpareja commented 11 years ago

Why do we abort on one-time script changes or on script errors?

As a user, I want to intervene/investigate if RH is unable to run the provided version of a script. My goal is to have all the provided scripts run at some point in time against the target database.

Why do we have one-time script directories?

As a user, I want to declare that some scripts should only be run once on a database because it is less work to write migration scripts that are not idempotent.

If someone tweaks a one-time script because they found a typo while defining a table, we can't run this against DBs that already received the original version of the script. The only way the script can be re-run is if it is modified to make it idempotent, or a manual change is made to the database to revert it to the state it would have been in if the script was never run.

If you specify a script should be run EVERYTIME, that script must be either idempotent or you don't care about the side-affects introduced by re-running the script. If you are changing an EVERYTIME script, the assumption is that your changes can still be run every time. I'm not sure what we would gain from preventing changes.

mpareja commented 11 years ago

BTW, if you both vote to close this issue as asinine, I won't object ;)

I think EVERYTIME scripts are a hack and would rather wait for customized workflows where you can define your own EVERYTIME directories.

ferventcoder commented 11 years ago

I respectfully disagree with EVERYTIME being a "hack". It's a convention that displays exactly what you expect, much like the ENV files.

But I do think 2 in the options above is what should happen. It sounds like right now it tries to run every time and complains if you have changed it. Right?

mpareja commented 11 years ago

Correct, the script is run every time and then RH complains on script change.

I would be happy with option 2 - it keeps things straightforward and consistent. (It is also in-line with the wiki documentation which mentions EVERYTIME scripts shouldn't be used in the UP directory.)

ferventcoder commented 11 years ago

let's file a bug on it then. :)

k4gdw commented 11 years ago

There already is one EVERYTIME directory. It's the permissions directory. Although it's clearly meant to contain scripts that manage permissions, there's technically no reason you couldn't put scripts that need to run every time in that folder. IIRC, there's a method to override the RH folder conventions so you might be able to rename the permissions folder to be more in line for what you'd be using it for. However, I'd prefer to put such scripts in one of the ANYTIME folders where it'd get run anytime there was a change or EVERYTIME if it was so tagged.

AlterDatabase, RunFirstAfterUpdate, Functions,Views, Sprocs, Indexes, RunAfterOtherAnyTimeScripts are all anytime folders where any script with EVERYTIME in it's name would always get run regardless of whether or not it was changed.

Bryan Johns K4GDW http://www.greendragonweb.com

Do not meddle in the affairs of dragons, for you are crunchy and taste good with ketchup.

On Fri, Jun 21, 2013 at 3:37 PM, Rob Reynolds notifications@github.com wrote:

I respectfully disagree with EVERYTIME being a "hack". It's a convention that displays exactly what you expect, much like the ENV files.

But I do think 2 in the options above is what should happen. It sounds like right now it tries to run every time and complains if you have changed it. Right?

— Reply to this email directly or view it on GitHub.

mpareja commented 11 years ago

You've opened up a can of worms - now I'll have to explain myself a bit further!

We use the ability to customize roundhouse directory names heavily. While deploying to a live site we actually perform multiple runs of roundhouse to make sure things are deployed in an appropriate order. We want to stay live in spite of database changes and even replication changes. For us, it is all about having finer grain control of the order things are done in. On one of our Roundhouse runs, we're applying replication changes using a One-Time directory. It is at that point in time that we needed the EVERYTIME script to be executed, so we tried tossing it in there and we were happy it worked. I was surprised we couldn't change the script, though. I would have expected it to not work at all (ala option 2) or for it to work like an EVERYTIME script would in all other directories.

wokket commented 8 years ago

G'day all,

Firstly, please let me know if you'd prefer me to open a new issue for this, but I felt that keeping the discussion in one place was important, and this was more recent than issue #35 .

We have been using RoundhousE in production for a dozen or so projects over the last couple of years and are loving it!

In the last week or so we have separately run into an issue in three of our projects, all related to the issue of changed RunOnce scripts. Having read the descriptions above of the intent of the up folder, reasons the behaviour was changed etc it doesn't seem this scenario has been addressed, so I'd like to:

A. describe our scenario, and B. Hopefully get a a solution for managing it using the existing tooling, or worst case: C. Make a case for reintroducing some sort of 'Skip just this file if it is a RunOnce but is different'

The Scenario

We're using Octopus Deploy to manage the configuration management, database settings etc for our products, and have included some Octopus variables in our RoundhousE scripts for things like SQL Login passwords, Sql service account names etc.

This allows us to have a single Sql script, run into different environments, with the configurations managed inside Octopus where it belongs. Everything runs perfectly until we restore a higher-environment database downlevel (eg Prod into Test etc)... when this occurs the next roundhouse deploy sees the prod roundhouse schema with the test files, which obviously have different hashes, and the scripts fail to deploy.

The UP scripts weren't written to be idempotent (they're meant to be RunOnce, and we love the safety net of preventing people changing them retrospectively) so using the -w flag to force them to run isn't a good solution.

Question the first: Is there a good way of managing this already that I haven't found?

Proposal

We add a per file setting similar to .EveryTime or .Env to indicate a file is possibly going to change for reasons that are out of our control, and those changes are to be ignored if the filename has been run previously - the file has been run in the past and that's the end of it regardless of content.

Options for naming we've currently thought of are: .IgnoreHashCheck - does what it says on the tin .DynamicContent - better reflects the scenario the flag is trying to fill .EnvSpecific -reflects the primary reason we're hitting this issue but is very close to .Env

I'm very open to better ideas here, good naming is a hard problem :)

This resolves the concerns around having to permanently run with -w, maintains the ethos that scripts should be RunOnce only, but allows for automated/continuous deployments and configuration management software to do its thing.

If there's no better answer, and people are open to this idea I'm happy to do the work and submit (my 2nd ever?) PR for the change.

Cheers, Tim

ferventcoder commented 8 years ago

@wokket So you want to have a file that may change, but you want to effectively ignore the change?

I think this is an interesting idea, but it also carries that the version number is no longer a global guarantee.

Thoughts on that?

ferventcoder commented 8 years ago

Sorry, more details - if you need to bring up a new environment then how do you guarantee that the file is the same as it was when it was run against all of the other environments?

wokket commented 8 years ago

@ferventcoder I've realised last night that RoundhousE already provides an example what we're trying to achieve for the exact same reasons: {{TokenReplacement}}

These are used to have a single file in source control, with different executed content at runtme to support environment specific values. These are also ignored if a database is restored from a different environment due to the hash check occurring prior to the token replacement.

We're looking for the exact same behaviour, but for files checked-in using tokens intended for other upstream tooling rather than RoundhousE...

The questions related to new environments, differences between environments, versioning etc are then all answered by the obvious answer "In the manner as TokenReplacement"... managing token-based scripts outside of Roundhouse obvioulsy requires care, and managing scripts with #{OctopusVariables} outside of an Octo Deploy requires the same care.

Should I update my proposal to include another (better?) naming convention: .Tokenised (.Tokenized for our US friends)

ferventcoder commented 8 years ago

Perhaps you should comment on the PR for custom token replacement?

ferventcoder commented 8 years ago

Because I think that may be what you are looking for.