-
Notifications
You must be signed in to change notification settings - Fork 360
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
make postgres connection string optional #948
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks!
How was this tested? (Can we add it to Nessie? Do we want to??)
db/migration.go
Outdated
m, err := migrate.NewWithSourceInstance("httpfs", src, params.ConnectionString) | ||
var m *migrate.Migrate | ||
if params.ConnectionString == "" { | ||
m, err = getMigrateUsingEnvVariables(src) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Trying to learn migrate...) This ends up calling NewWithInstance
, the other branch calls NewWithSourceInstance
. Can't we do the same in both and have a smaller getMigrateUsingEnvVariables
?
[Not in this PR, of course...] |
Codecov Report
@@ Coverage Diff @@
## master #948 +/- ##
==========================================
- Coverage 44.44% 44.43% -0.02%
==========================================
Files 143 143
Lines 11486 11487 +1
==========================================
- Hits 5105 5104 -1
- Misses 5744 5746 +2
Partials 637 637
Continue to review full report at Codecov.
|
db/migration.go
Outdated
@@ -107,7 +105,9 @@ func getMigrate(params params.Database) (*migrate.Migrate, error) { | |||
defer func() { | |||
_ = src.Close() | |||
}() | |||
|
|||
if params.ConnectionString == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you do this one on a temp var so we will not modify the caller data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool!
No description provided.