implement additional context specific sql interfaces#921
implement additional context specific sql interfaces#921kylejbrock wants to merge 2 commits intolib:masterfrom
Conversation
reorganize imports Move context methods per PR feedback fix formatting
|
This failed with: Looks like this has bugs. |
| } | ||
|
|
||
| func (st *stmt) watchCancel(ctx context.Context) func() { | ||
| if done := ctx.Done(); done != nil { |
There was a problem hiding this comment.
Spidey sense says this is problematic. Way too many go routines and chan listens to make me feel safe here.
|
Yea. I’ve not had time to look into why. I tried testing it locally and it passed. So, it’ll require some deeper investigation.
… On Dec 10, 2019, at 3:16 PM, Matt Jibson ***@***.***> wrote:
This failed with:
=== RUN TestContextCancelBegin
TestContextCancelBegin: go18_test.go:212: unexpected error: pq: current transaction is aborted, commands ignored until end of transaction block
--- FAIL: TestContextCancelBegin (0.02s)
Looks like this has bugs.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#921?email_source=notifications&email_token=ABG7PMWEYS4TORKBBQVVM73QYABJ5A5CNFSM4JULIIIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGQ7JIA#issuecomment-564262048>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABG7PMVCZXGX6AHYH7FBCPDQYABJ5ANCNFSM4JULIIIA>.
|
|
You may need to stress it locally. Could be a race condition. |
|
Oops, ignore that PR I linked against this one. I meant to link to a different one. |
The failure here seems unrelated to this PR. I spent some time trying to reproduce the failure and was unable to do so with either this PR or master. I did find one other failure for this test on Travis, https://travis-ci.org/lib/pq/jobs/355557649, so maybe the test itself has a race.
I agree that this code looks scary! But it's the same code as used here https://github.com/lib/pq/blob/master/conn_go18.go#L90 I have been test-driving this PR on a real project in a test environment, and I also wrote a couple of toy servers to check that the context cancellation works as I expect. Everything so far looks good 👍 |
Got rid of OpenConnector for simplicity's sake. Noticed "pq: current transaction is aborted" in go18_test.go:212, but it doesn't seem to be related. The same issue popped up in lib#921.
Lorena-babygirl
left a comment
Lorena-babygirl
left a comment
There was a problem hiding this comment.
submit tour feedback without any explicit approval.
5 participants
No description provided.