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

diesel migration run panicked #140

Closed
weiznich opened this Issue Jan 26, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@weiznich
Contributor

weiznich commented Jan 26, 2016

$ ls migrations/20160126232310_add_foo    
down.sql  up.sql  up.sql~
$ diesel migration run    
thread '<main>' panicked at 'called `Result::unwrap()` on an `Err` value: MigrationError(UnknownMigrationFormat("migrations/20160126232310_add_foo"))', ../src/libcore/result.rs:688

It seems like diesel migration run is panicking if there are additional files in the migration dir.
(Workaround: Delete those files).

weiznich added a commit to weiznich/diesel that referenced this issue Jan 26, 2016

Fix issue diesel-rs#140
Simply check if up.sql and down.sql exists. Ignore all other files.
@mfpiccolo

This comment has been minimized.

Collaborator

mfpiccolo commented Jan 26, 2016

This has been discussed here #64 and it was determined that only files with a leading . would be ignored.

@weiznich

This comment has been minimized.

Contributor

weiznich commented Jan 26, 2016

*~ are recovery files created by most editors. I think such files should be allowed/ignored. Furthermore there may be other examples(emacs uses #filename# for example).
I see three possibilities here:

  1. maintain a list of "allowed" file endings
  2. simply check if there is a down.sql and a up.sql in the migrations dir
  3. remove all "temporary" files from the migrations dir
@mfpiccolo

This comment has been minimized.

Collaborator

mfpiccolo commented Jan 26, 2016

I don't like the idea of removing files from the directory. The maintained list seems reasonable since we can add known files like the ones you mentioned and issues will be opened for other use-cases that come up. Option 2 seems to be reasonable to me as well.

Were there other reasons aside from keeping these dirs clean for not allowing other files? @sgrif

@sgrif

This comment has been minimized.

Member

sgrif commented Jan 27, 2016

I think the most "correct" thing to do here is to ignore files that are ignored by VCS, but that feels a bit like overkill. I'm fine with just making sure up.sql and down.sql are there. We should also make sure that we better communicate our errors instead of just panicing

weiznich added a commit to weiznich/diesel that referenced this issue Jan 27, 2016

Fix issue diesel-rs#140
Simply check if up.sql and down.sql exists. Ignore all other files.

weiznich added a commit to weiznich/diesel that referenced this issue Jan 30, 2016

Fix issue diesel-rs#140
Simply check if up.sql and down.sql exists. Ignore all other files.

sgrif added a commit that referenced this issue Jan 30, 2016

@sgrif

This comment has been minimized.

Member

sgrif commented Jan 30, 2016

Fixed by #141

@sgrif sgrif closed this Jan 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment