Allow the ability to pass timezone when generating migration files #509
Conversation
|
/cc @dhui |
| @@ -180,7 +183,12 @@ Database drivers: `+strings.Join(database.List(), ", ")+"\n", createUsage, gotoU | |||
| log.fatal("error: -ext flag must be specified") | |||
| } | |||
|
|
|||
| if err := createCmd(*dirPtr, startTime, *formatPtr, name, *extPtr, seq, seqDigits, true); err != nil { | |||
| timezone, err := time.LoadLocation(*timezoneName) | |||
There was a problem hiding this comment.
Let's avoid loading TZ data if the named tz is the default. That way, missing tz data won't break the CLI if a tz isn't specified.
There was a problem hiding this comment.
@dhui on another thought, I cannot see how this will break, since the default location will be UTC even if you do not specify the timezone. Essentially we will do time.LoadLocation("UTC")
With timezone
go run ./cmd/migrate/... create -tz Europe/Athens -ext postgres hello
/Users/nronas/projects/migrate/20210223204225_hello.up.postgres
/Users/nronas/projects/migrate/20210223204225_hello.down.postgres
Without timezone
go run ./cmd/migrate/... create -ext postgres hello_no_tz
/Users/nronas/projects/migrate/20210223184235_hello_no_tz.up.postgres
/Users/nronas/projects/migrate/20210223184235_hello_no_tz.down.postgres
With unknown timezone
go run ./cmd/migrate/... create -tz Asgard -ext postgres hello_invalid_tz
unknown time zone Asgard
exit status 1
There was a problem hiding this comment.
The issue is when the tz database is missing, not when the -tz option is not specified or invalid/incorrect.
From the time.LoadLocation() docs:
The time zone database needed by LoadLocation may not be present on all systems, especially non-Unix systems. LoadLocation looks in the directory or uncompressed zip file named by the ZONEINFO environment variable, if any, then looks in known installation locations on Unix systems, and finally looks in $GOROOT/lib/time/zoneinfo.zip.
There was a problem hiding this comment.
Oh, I completely misinterpreted what you were referring to. I think we should still be fine since zoneinfo does a look upon an IANA file only if the location name is neither 'UTC' nor 'Local' https://github.com/golang/go/blob/master/src/time/zoneinfo.go#L630-L635
There was a problem hiding this comment.
Thanks for digging deeper and discovering that the tz db isn't loaded if the location UTC or Local are specified! Looks like this behavior has been around since at least Go 1.4 and seems unlikely to change in the future, so I think the current approach is fine. The fix is easy enough to make should this break in the future.
Pull Request Test Coverage Report for Build 145
💛 - Coveralls |
Implements: #503
Adds a new
-tzcmd option oncreatethat allows specifying the timezone on which non-sequential migration files will be generated with.