Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using DB with an already existing socket or connection #162

Closed
will opened this issue Feb 17, 2022 · 6 comments
Closed

Using DB with an already existing socket or connection #162

will opened this issue Feb 17, 2022 · 6 comments

Comments

@will
Copy link

will commented Feb 17, 2022

I want to make it possible to use the pg driver with an SSH2::Channel. It's possible, I have it working by using the pg driver directly without crystal-db, and with monkey patching PQ::Connection to take the ssh channel as a socket. So that's all cool, but it'd be nice to also use crystal-db.

In general it may also be nice to let people establish their own tcp socket ahead of time and pass it in. Maybe?

I think for these uncommon cases it'd be okay to make people create a postgres connection manually, and pass it into crystal-db to wrap. This may already be possible and I just haven't figured out how yet. If not what do you think the best way to do this would be?

@will
Copy link
Author

will commented Apr 3, 2023

Ok I think I have this working in a promising way. Basically it just needs to store one proc that returns an IO in a DB::Database, then the rest can be handled by crystal-pg.

Here is it all working: https://gist.github.com/will/81651b17c267d34dcde5faefec1e0ff9

require "pg"
require "ssh2"

# Add an initlizer that directly sets the socket
class PQ::Connection
  def initialize(@soc, @conninfo)
  end
end

# Add a new crystal-pg database context that can store a proc that returns a
# socket for the new PQ::Connection initalizer
class DB::TunneledDatabase < DB::Database
  getter setup_tunnel : Proc(IO)

  def initialize(@setup_tunnel : Proc(IO), driver : Driver, uri : URI)
    super(driver, uri)
  end
end

module PG
  # Add a new connection class that is mostly the same as PG::Connection,
  # except it calls the setup tunnel proc to get the socket
  class TunneledConnection < Connection
    def initialize(@context)
      @prepared_statements = context.prepared_statements?
      @connection = uninitialized PQ::Connection

      begin
        conn_info = PQ::ConnInfo.new(context.uri)
        channel = context.setup_tunnel.call                  # <=== new line
        @connection = PQ::Connection.new(channel, conn_info) # <=== changed line
        @connection.connect
      rescue ex
        raise DB::ConnectionRefused.new(cause: ex)
      end
    end
  end

  # Add a new crystal-db driver for the new connection class
  class TunneledDriver < ::DB::Driver
    def build_connection(context : ::DB::ConnectionContext) : Connection
      TunneledConnection.new(context)
    end
  end
end



# ssh into the postgres server 
session = SSH2::Session.connect("p.someid.db.postgresbridge.com", 22)
session.login_with_pubkey "username", "./key", "./key.pub"

# create a proc that returns an IO talking to the postgres socket
connect_proc = -> do
  channel = session.open_session
  channel.command("nc -U /var/run/postgresql/.s.PGSQL.5432")
  channel.as IO
end


# instead of DB.open, start the connection manually since that's the only way
# to pass in the new proc
db = DB::TunneledDatabase.new(
  setup_tunnel: connect_proc,
  driver: PG::TunneledDriver.new,
  # the host part is ignored, but user and database name all work normally
  uri: URI.parse("postgres://my_pg_user@whatever/db_name")
)

p db.query_one("select current_user || now()", &.read)
db.close

It'd maybe be nice if DB.open could take that proc directly though? I'm not sure the best interface though.

@bcardiff
Copy link
Member

bcardiff commented Apr 3, 2023

I think that allowing DB.open to receive an IO factory proc make sense. It would also make crystal-lang/crystal-mysql#14 possible without patching.

What make noise to me is that not all drivers works with IO. sqlite3 and others backed by C libraries don't need IO at all. But having this flexibility make sense.

Make DB.open("pg://server/db?options..", build_io) would make the hostname probably redundant int the URI. I've also been thinking of allowing setup of databases without URI, but it complicates a unified API across all drivers.

I think we can move forward and allow a io factory, but before:

  • should we model lifecycle a bit more in order to do setup/teardown of anything that should happen before the first io and after the last one that is closed?
  • can we offer some recovery as we do know with broken connections with the underlying resource?

@will
Copy link
Author

will commented Apr 4, 2023

Would it help things any if DB.open took some new struct/class in addition to URI|String ? That new thing could maybe be subclassed by each driver to have the fields it needs, like this IO factory proc, and whatever else.

@straight-shoota
Copy link
Member

I think a generic configuration interface as @will suggests might be a good idea for a clean and extensible API.

@bcardiff
Copy link
Member

You mean to add some api to create databases with handmade connection objects and avoid requiring a connection string directly? That would be the more elementary api I would think of.

The configurations for the pool should still be given somehow.

And I would like to keep the connect/open with the same api, but is not a must.

@bcardiff
Copy link
Member

Let me know what you think about #181

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants