Skip to content
Snippets Groups Projects
Unverified Commit 37ebb437 authored by dpsutton's avatar dpsutton Committed by GitHub
Browse files

Ensure we use the ssh tunnel on action execution (#29228)

* Ensure we use the ssh tunnel on action execution

I'm gonna see if we have an existing way to run tests against ssh dbs. I
see there are some tests that use a faking ssh server. Not sure if those
are useful or not. But otherwise ssh stuff needs out of process help to
accomplish so not sure if it's easily feasible right now

* introduce new `with-unpooled-connection-spec` macro

it is identical to the `with-connection-spec-for-testing-connection`
macro but has a better name. In the future they could diverge as testing
requirements and action requirements change. But we want that
_implementation_ (which correctly uses tunnels), but we do not want a
"testing connection" slug in the usage.

In the future (or now if we prefer a different name) we can rename it,
find the usages that are not testing vs the ones that are, etc.

* tests for actions over ssh

* unused require of mb.u.ssh

had brought it in but then stuffed all of this inside of
`with-unpooled-connection-spec` so no need to worry about ssh, closing
it, ensuring the port is there, etc.

* Don't test h2 with ssh tunnel

details lack a host and it npe's in the ssh tunnel stuff. No big loss as
ssh on h2 is not supported (due to this error, and probably many more)

* Switch to toucan2 for db access in tests

* Use connection pool for custom actions connection

Was originally using `jdbc/get-connection` from the details of the
db. This was a one-off non-pooled connection that also did not reuse ssh
tunnels. I thought the one-off connection was important since it is not
read only, but that's not the case. So we can reuse the exact same
mechanism that the implicit actions are using with
`metabase.driver.sql-jdbc.actions/with-jdbc-transaction`.

* cyclic dependency

almost moved it into execute, but that gives a very inviting
`sql-jdbc.execute/with-jdbc-transaction`. Welcoming even. But
`sql-jdbc.actions/with-jdbc-transaction` lets you know its only for
transactions.
parent cf662f31
No related branches found
No related tags found
No related merge requests found
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment