Support sql driver connector

[ysdanielkim]

What changed?

SQL config now supports passing driver.Connector instance.

Why?

It's currently not possible to embed a Temporal server to a host application where the database connection is managed by the host application and is provided as a database connector. This allows the host application to pass a driver connector instance rather than the the individual sql config strings.

How did you test it?

I have embedded this code into a custom application and verified that the driver connector instance is applied properly.

Potential risks

This is a non-breaking change with very minimal risks.

Is hotfix candidate?

No.

[CLAassistant]

All committers have signed the CLA.

[github-actions[bot]]

This PR was marked as stale. Please update or close it.

[ysdanielkim]

I left a comment on the correctness of your postgres changes. I don't like adding this to config.SQL as that's all coming from YAML and this feels a bit messy, but I don't have a better answer right now (though I'll think about it).

I agree. Thank you for thinking about it. I'd be happy to make any necessary changes.

[tdeebswihart]

So with https://github.com/temporalio/temporal/pull/5926 we're about to automatically reconnect on connection errors. Rather than provide the driver connection itself, we need you to provide a function that will connect on demand.

We're implementing this to deal with, for example, Aurora RDS failovers

[tdeebswihart]

I left a comment on the correctness of your postgres changes. I don't like adding this to config.SQL as that's all coming from YAML and this feels a bit messy, but I don't have a better answer right now (though I'll think about it).

I agree. Thank you for thinking about it. I'd be happy to make any necessary changes.

If I figure something out I'll let you know, otherwise I can refactor it later once I've figured it out :)

[ysdanielkim]

So with #5926 we're about to automatically reconnect on connection errors. Rather than provide the driver connection itself, we need you to provide a function that will connect on demand.

We're implementing this to deal with, for example, Aurora RDS failovers

Okay. So something like this?

type SQL struct {
  // DatabaseConnection returns a new database connection. Using this will bypass the string based data source.
  DatabaseConnection func() (*sqlx.DB, error)
  ...
}

[tdeebswihart]

So with #5926 we're about to automatically reconnect on connection errors. Rather than provide the driver connection itself, we need you to provide a function that will connect on demand. We're implementing this to deal with, for example, Aurora RDS failovers

Okay. So something like this?

type SQL struct {
  // DatabaseConnection returns a new database connection. Using this will bypass the string based data source.
  DatabaseConnection func() (*sqlx.DB, error)
  ...
}

That would be perfect, though I'd name it something like Connect or something. I've merged the PR I referenced; you may wish to rebase/merge it in. I'm happy to help if you need it

[tdeebswihart]

@ysdanielkim can you add ,json:"-" to the struct tag for this new field? We need to make sure it's ignored by the JSON serializer. We do something messy when using these in our tests, it looks like

[ysdanielkim]

@tdeebswihart thank you for guiding me through! Looks like I don't have write access. Please merge this for me.

[tdeebswihart]

Done! Thanks for your contribution :)