Fluentmigrator: FluentMigrator throws for HasMigrationsToApplyUp

[thijstijsma]

I'm a new user to FluentMigrator and got an exception when executing MigrateUp() telling me there are no migrations. I can understand that flow.

So I look at the documentation and find the method HasMigrationsToApplyUp() to check whether or not there are any migrations to apply. To my surprise this also throws.

I see that this exception was added in https://github.com/fluentmigrator/fluentmigrator/commit/fe7740dc404d05643f8287da094ae480e48607b3, because of #670 among others.

I want to raise the question whether or not that's the right place to throw the exception, what if I just want to know what amount or types of migrations I have before taking action? Maybe it should be higher up the call chain, so the console (or where you actually want to throw) can deal with it.

[fubar-coder]

The reasoning behind this exception is that it is usually a configuration problem when no migration could be found at all - usually on every path that enumerates the available migrations.

You can still convince me 😁 . What is your reasoning why HasMigrationsToApplyUp shouldn't throw when MigrateUp will throw an exception?

[eloekset]

I think HasMigrationsToApplyUp should return false instead of throw when there's no migrations to apply. The reason to call this method is to find whether there are migrations to apply or not, and getting an exception instead of return value false is unexpected.

The unit test in my PR, where all the HasMigrationsToApply methods were added, also tests that the method returns false: https://github.com/fluentmigrator/fluentmigrator/pull/701/files#diff-9b2a760dba59797dfba7c97840c3c475R283

The reason why I added HasMigrationsToApply is described in issue #700. And the code snippet will fail if the method throws.

[fubar-coder]

My reasoning was that HasMigrationsToApply should throw, because choosing the false path might cause more problems than the exception.

Example:

• Refactoring of the code base
• FluentMigrator cannot find migrations
• HasMigrationsToApply was called and the application thinks that everything is up-to-date (even though it's not)
• Some time later, the application tries to access a new database field, that didn't get applied, because HasMigrationsToApply just returned false

I'd prefer having an exception directly at application start instead of giving the impression that everything is OK.

EDIT: Issue #700 is indeed the only situation I can imagine where it'd be fine if HasMigrationsToApply can return false. Maybe we should think about making its return value nullable?

[eloekset]

I can't see why making the return value nullable will improve anything. The current behavior is that it either returns true or throws. So actually, for the current behavior the method should be renamed to EnsureHasMigrationsToAppy and get return value void.

If the current behavior is desirable, I think a new method named EnsureHasMigrationsToApply should be added with the current behavior and HasMigrationToApply should be reverted to it's original behavior.

[fubar-coder]

Nah, it can return false when at least one migration (regardless if it was applied or not) was found.

EDIT: Just to clarify: The exception is only thrown when no migrations could be found in the code base at all - independent of its status (applied or not).

[eloekset]

Oh, in that case I agree that the current behavior is better than the original. No reason to include FluentMigrator in a project unless you've also got an assembly with migration steps.

[thijstijsma]

My apologies for my belated reply, my son was born a few weeks ago!

Thank you all for the clarification about the reasoning that HasMigrationToApply throws when there are no migrations at all. I agree that the scenario's provided by @fubar-coder are too important to not throw there.