postgres binding, ping on init #3595
Conversation
Signed-off-by: Fabian Martinez <[email protected]>
bindings/postgres/postgres.go
Outdated
| connCtx, connCancel := context.WithTimeout(ctx, m.Timeout) | ||
| p.db, err = pgxpool.NewWithConfig(connCtx, poolConfig) | ||
| connCancel() |
There was a problem hiding this comment.
| connCtx, connCancel := context.WithTimeout(ctx, m.Timeout) | |
| p.db, err = pgxpool.NewWithConfig(connCtx, poolConfig) | |
| connCancel() | |
| connCtx, connCancel := context.WithTimeout(ctx, m.Timeout) | |
| defer connCancel() | |
| p.db, err = pgxpool.NewWithConfig(connCtx, poolConfig) |
There was a problem hiding this comment.
we have this exact same code in the postgres state store
bindings/postgres/postgres.go
Outdated
| pingCtx, pingCancel := context.WithTimeout(ctx, m.Timeout) | ||
| err = p.db.Ping(pingCtx) | ||
| pingCancel() |
There was a problem hiding this comment.
| pingCtx, pingCancel := context.WithTimeout(ctx, m.Timeout) | |
| err = p.db.Ping(pingCtx) | |
| pingCancel() | |
| pingCtx, pingCancel := context.WithTimeout(ctx, m.Timeout) | |
| defer pingCancel() | |
| err = p.db.Ping(pingCtx) |
bindings/postgres/postgres.go
Outdated
| p.db, err = pgxpool.NewWithConfig(ctx, poolConfig) | ||
| connCtx, connCancel := context.WithTimeout(ctx, m.Timeout) | ||
| p.db, err = pgxpool.NewWithConfig(connCtx, poolConfig) | ||
| connCancel() |
There was a problem hiding this comment.
Was this meant to be deferred? Wouldn't we still need the p.db connection for the ping below?
There was a problem hiding this comment.
Ha! Josh was faster :D
There was a problem hiding this comment.
we have this exact same code in the postgres state store
|
Can you also please see if it's possible to add a test for this, possibly following this pattern? https://github.com/dapr/components-contrib/pull/3285/files#diff-6bd238ae78b3ac8a1e9d990dab894d9a9e90a402f52adfdc605c311ae8728712 |
Signed-off-by: Fabian Martinez <[email protected]>
Signed-off-by: Fabian Martinez <[email protected]>
|
done all |
|
ping @yaron2 |
|
Linter is failing. otherwise LGTM |
Signed-off-by: Fabian Martinez <[email protected]>
|
updated |
Description
Implementation for #3594
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: