gosnowflake icon indicating copy to clipboard operation
gosnowflake copied to clipboard

Published 20 hours ago • verified publisher icon snowflakedb

Set the result type before calling exec from Ping

Open nyergler opened this issue 3 years ago • 3 comments

Description

If the context has been set to Async Mode, the driver will attempt to inspect the result type. If the result type has not been set, the driver will panic when returning the [partial] result.

We ran into this issue when pinging a connection to test for aliveness when the context had WithAsyncMode applied. While async mode doesn't make a lot of sense for pings, in this case the same context is used to ping before executing a query.

Happy to write a test, just wasn't clear to me where Ping tests would live.

Checklist

  • [ ] Code compiles correctly
  • [x] Run make fmt to fix inconsistent formats
  • [x] Run make lint to get lint errors and fix all of them
  • [ ] Created tests which fail without the change (if possible)
  • [ ] All tests passing
  • [ ] Extended the README / documentation, if necessary

nyergler avatar Nov 30 '21 20:11 nyergler

Hi @nyergler , yes would you mind writing a test for this? Thank you.

sfc-gh-jbahk avatar Dec 07 '21 18:12 sfc-gh-jbahk

@sfc-gh-jbahk Of course; any chance you can point me to the right place for that to live? Wasn't immediately clear to me how Ping is currently being tested.

nyergler avatar Dec 07 '21 20:12 nyergler

I think connection_test.go should suffice. Its tests are in driver_test.go at the moment, but I think it makes sense to move it under the former.

sfc-gh-jbahk avatar Dec 07 '21 20:12 sfc-gh-jbahk

Hello @nyergler ! Are you planning to rebase and provide a test for your solution? I'd like to merge it when it is ready and tested :)

sfc-gh-pfus avatar Oct 17 '23 08:10 sfc-gh-pfus

Thanks for the nudge, @sfc-gh-pfus ; I'll take a look at that this week.

nyergler avatar Oct 17 '23 23:10 nyergler

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

github-actions[bot] avatar Oct 19 '23 15:10 github-actions[bot]

I have read the CLA Document and I hereby sign the CLA

nyergler avatar Oct 19 '23 15:10 nyergler

@sfc-gh-pfus Updated.

nyergler avatar Oct 19 '23 15:10 nyergler

I have read the CLA Document and I hereby sign the CLA

jc avatar Oct 19 '23 16:10 jc

recheck

nyergler avatar Oct 19 '23 20:10 nyergler

Hello @nyergler ! I approved your PR, but in the meantime a colleague of mine merged his PR. Since this is cross-repo PR I cannot rebase it myself. Can you rebase?

sfc-gh-pfus avatar Oct 20 '23 09:10 sfc-gh-pfus

@sfc-gh-pfus Done.

nyergler avatar Oct 20 '23 14:10 nyergler

Thank you very much @nyergler ! I merged your PR.

sfc-gh-pfus avatar Oct 23 '23 05:10 sfc-gh-pfus