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

Should grate failed if a folder doesn't exist? #292

Open
MangelMaxime opened this issue Jan 10, 2023 · 8 comments
Open

Should grate failed if a folder doesn't exist? #292

MangelMaxime opened this issue Jan 10, 2023 · 8 comments
Labels
question Further information is requested

Comments

@MangelMaxime
Copy link
Contributor

MangelMaxime commented Jan 10, 2023

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
Copy link
Owner

erikbra commented Feb 13, 2023

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?

@erikbra erikbra added the question Further information is requested label Feb 13, 2023
@MangelMaxime
Copy link
Contributor Author

@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
Copy link
Owner

erikbra commented Feb 27, 2023

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

@MangelMaxime
Copy link
Contributor Author

MangelMaxime commented Feb 27, 2023

@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
Copy link

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
Copy link
Contributor Author

@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
Copy link
Owner

erikbra commented Mar 28, 2023

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
Copy link
Contributor Author

@erikbra For me this work yes 👍

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

No branches or pull requests

3 participants