erikbra/grate

Should grate failed if a folder doesn't exist?

Opened this issue · 8 comments

MangelMaxime commented

Describe the bug

If a folder is invalid Grate "just" skip it and return a successful error code.

To Reproduce

Run dotnet grate --connectionstring "XXX" --folders thisFolderDoesnTExist --noninteractive and see that print:

Initializing connections.
Running grate v1.4.0.0 against  - antidote_tenant_catalog.
Looking in . for scripts to run.
================================================================================
Setup, Backup, Create/Restore/Drop
================================================================================
================================================================================
Grate Structure
================================================================================
================================================================================
Versioning
================================================================================
 Migrating antidote_tenant_catalog from version 0.0.0.0 to 0.0.0.1.
 Versioning antidote_tenant_catalog database with version 0.0.0.1.
================================================================================
Migration Scripts
================================================================================
Skipping './Server/Antidote.TenantCatalog.Database/Migrations/', ./Server/Antidote.TenantCatalog.Database/Migrations/ does not exist.


grate v1.4.0.0 has grated your database (antidote_tenant_catalog)! You are now at version 0.0.0.1. All changes and backups can be found at "/Users/mmangel/.local/share/grate/migrations/antidote_tenant_catalog/2023-01-10T16_30_09.5450510_01_00".

Expected behavior
A clear and concise description of what you expected to happen.

I think Grate should fail if an expected folder doesn't exist. It took me 30min to understand why I didn't have any tables created in my database even if Grave was saying all is fine.

It was both due to the fact that an invalid folder is not an error and that the output was green (tracked here #282)

Screenshots
If applicable, add screenshots to help explain your problem.

image

Desktop (please complete the following information):

  • OS: macOS
  • Version: 13.0.1 (22A400)
erikbra commented

This is as expected. The default configuration is a fixed set of folders, but it would be a bit demanding to require that all of the folders exist, even if they do not make sense for a particular user (see https://erikbra.github.io/grate/folder-configuration/#default-folder-configuration).

It might be an idea to introduce a --strict, --validate, --fail-on-missing-folders option, if there are more people needing it. Do you think this would be a useful, much used feature?

MangelMaxime commented

@erikbra I understand that grate by default as some pre defined folders. When doing dotnet grate --folders <path to a folder> doesn't the users say all the files/folders should be under <path to a folder>?

If yes, I think it is a good idea to warn the user that nothing was found. To me it was not immediately obvious that I mess up something, perhaps do to the fact that grate is kind of verbose and also everything was in green which kind of biased me :).

If I need to use a flag to activate this behaviour because it is against grate philosophy, I will take that for sure :)

erikbra commented

I do see your point, @MangelMaxime. Maybe we could output a warning if the verbosity level is one of the higher ones?

MangelMaxime commented

@erikbra Having the using need to increase the verbosity level means that this is not "supported" out of the box.

And I think that it is a bit too indirect to hide this report behind the verbosity level. Personally, I almost never increase verbosity level of the tool I use.

If the choice is between hiding the feature behind a verbosity level or having a dedicated flag. I think having the dedicated flag is better in term of discoverability.

willsmith9182 commented

I often use RoundhousE (the predecessor) against partial folder strcutures, sometimes our db repos don't have certain folders. I'm planning to migrate to grate, and this would be a no deal for me if it erros whena single follder is missing.

I like the idea of flags to validate, but only if it's actually a valid requirement.

MangelMaxime commented

@willsmith9182 IHMO there is a difference in:

  • not having folders because they are from the default convention, in that case no errors should be generated
  • not having a folder when the user explicitly said it should exist, in that case it should error

From what I understood unfortunately the second case is not inlined with Grate philosophy. Therefore, the idea of ussing --strict flag to opt-in in generating errors if a folder is missing when asked by the user.

But indeed, the idea is to keep the compatibility with the predecessor project.

erikbra commented

To sum up, @MangelMaxime and @willsmith9182 - would this approach work for you both?

  • If the default folders are used, just skip folders that do not exist
  • If specifying custom folders, fail if any of the folders do not exist
MangelMaxime commented

@erikbra For me this work yes 👍

  • 👍2