add statement timeout query param for postgres #327
Conversation
Pull Request Test Coverage Report for Build 648
💛 - Coveralls |
database/postgres/postgres.go
Outdated
| @@ -25,6 +26,7 @@ func init() { | |||
| } | |||
|
|
|||
| var DefaultMigrationsTable = "schema_migrations" | |||
| var DefaultStatementTimeoutMs = 10000 // 10 seconds | |||
There was a problem hiding this comment.
this is actually no longer needed
There was a problem hiding this comment.
In other words, If the caller
- does not pass any value, we default to 0 (i.e background context)
- passes an invalid value, we return error from Atoi and abort migration
- passes valid value, we use it as the timeout value (i.e context with timeout)
Co-Authored-By: Roopak Venkatakrishnan <roopak.v@gmail.com>
psuresh39
force-pushed
the
psuresh/postgres-statement-timeout
branch
from
96f63e8
to
87c2491
Compare
|
@dhui Can you give this another read? I would like to merge this soon. |
…to psuresh/postgres-statement-timeout
| if statementTimeoutString != "" { | ||
| statementTimeout, err = strconv.Atoi(statementTimeoutString) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
@dhui wondering if we should throw a custom error here instead of the Atoi one, thoughts?
There was a problem hiding this comment.
I think we'd lose information w/ a custom error, so it'd be cleaner to wrap the error using multierror.
not urgent. please go ahead and merge this when you can. |
|
thanks @dhui . When do you plan to make a new release? |
|
The earliest would be in a week so the change can be tested by people running the master branch. If you'd like to use the change, I'd use the master branch for now and change to the next release when it's available. |
Adding statement timeout option to postgres driver as a query param