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

Create migration file with next version number #50

Merged
merged 1 commit into from
Jun 19, 2017

Conversation

VojtechVitek
Copy link
Collaborator

Fixes #25

@VojtechVitek VojtechVitek merged commit d947738 into master Jun 19, 2017
@VojtechVitek VojtechVitek deleted the create_sequential branch June 19, 2017 21:30
@mschmo
Copy link

mschmo commented Jun 19, 2017

This is a fairly large non-backwards compatible update for those of us who have been using timestamps from the original create behavior. The version should be updated to 2.0.

@VojtechVitek
Copy link
Collaborator Author

Hi @mschmo, how is it non-backward compatible for you? Is it just that you invoke goose create and you expect new timestamp to be created, instead of last.version++?

Does it goose create work for you, or does it explode?

I strongly advice you guys switch to sequential numbers instead of timestamps to prevent version mismatch, see #25.

But yeah, if there's more people thinking we should release v2.0 because of this change, we can totally do that.

@mschmo
Copy link

mschmo commented Jun 20, 2017

I can see the benefits of using sequential numbering and will consider updating our current migration process to use that instead of timestamps, but that does not change the fact that this is change breaks things.

goose create does not work as expected. Here is one example of a few errors in version bumping that I have noticed:

20170613141552_old_timestamp_versioning_sql.sql
20170613141553_new_sequential_versioning_sql.sql
20170613141555_new_sequential_versioning_go.go
20170614115105_old_timestamp_versioning_go.go

After updating to this commit's change the sequential versions, last.Version+1 did not build off the oldest timestamp in that migrations directory (the new sequential versions should have started at 20170614115106).

This does seem like it may just a bug that could be related to sql vs go migrations, in which case this should be reverted and fixed before you attempt to make a change like this, or the version should be bumped so that I can stick to the v1.0 of this package while it is fixed.

@VojtechVitek
Copy link
Collaborator Author

@mschmo let me try

@VojtechVitek
Copy link
Collaborator Author

@mschmo

$ cd example

$ go run *.go sqlite3 ./foo.db status
    Applied At                  Migration
    =======================================
    Pending                  -- 20170613141552_first.sql
    Pending                  -- 20170614115105_second.go

$ go run *.go sqlite3 ./foo.db create hello_there sql
Created new file: 20170614115106_hello_there.sql

$ go run *.go sqlite3 ./foo.db status
    Applied At                  Migration
    =======================================
    Pending                  -- 20170613141552_first.sql
    Pending                  -- 20170614115105_second.go
    Pending                  -- 20170614115106_hello_there.sql

It works as expected. It bumped the last (highest) timestamp by one, as expected.

@mschmo am I missing anything?

@mschmo
Copy link

mschmo commented Jun 20, 2017

Is this expected behavior for you for a directory that already has timestamped migrations?

$ ls -l
20170620123456_first.sql
20170620123457_second.go

$ goose create third
Created new file: 20170620123457_third.go

$ ls -l
20170620123456_first.sql
20170620123457_third.go
20170620123457_second.go

Perhaps I am misunderstanding how to create migration files, but I feel as though that 3rd migration should be 20170620123458.

@mschmo
Copy link

mschmo commented Jun 20, 2017

Another example that doesn't make sense to me:

$ goose create first
Created new file: 00001_first.go

$ goose create second
Created new file: 00001_second.go

$ goose create third sql
Created new file: 00001_third.sql

$ goose create fourth go
Created new file: 00002_fourth.go

@VojtechVitek
Copy link
Collaborator Author

Yea, this is not right. The example I provided above doesn't have this bug. It increments versions correctly.

I wonder what's the difference here.. Can you pls update your goose pkg and rebuild your binary?

@mschmo
Copy link

mschmo commented Jun 20, 2017

Ok I think I'm now identifying this as an issue related to not supplying a sql|go option with goose create, which I believe is suppose to default to go.

I can explore a little more and open an issue if necessary sometime later today. Thanks for checking on this!

@VojtechVitek
Copy link
Collaborator Author

VojtechVitek commented Jun 20, 2017

$ goose create first
Created new file: 00001_first.go

# you'd have to rebuild your goose binary here
# note that Go migrations must be built-in,
# otherwise the custom goose binary can't see such migrations

$ goose create second
Created new file: 00001_second.go

# rebuild again

$ goose create third sql
Created new file: 00001_third.sql

$ goose create fourth go
Created new file: 00002_fourth.go

# rebuild again

@VojtechVitek
Copy link
Collaborator Author

@mschmo will be fixed in #55. Pls try it out.

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

Successfully merging this pull request may close these issues.

2 participants