streamline the database interfaces and add pgx adapter #29
Conversation
pkg/sql/sql_adapter.go
Outdated
| } | ||
|
|
||
| // BeginTx converts the stdSQL.Tx struct to our Tx interface | ||
| func (c *StdSQLBeginner) BeginTx(ctx context.Context, options *sql.TxOptions) (Tx, error) { |
There was a problem hiding this comment.
How about making these value receivers instead of pointers? There should be no change performance-wise since the embedded values are pointers anyway, and it would make for a bit simpler construction of these. :)
|
Thanks for contributing @julesjcraske and I'm sorry it took us so long to reply! This looks great, and I since we're getting more people asking about using specific database frameworks with the outbox pattern, I'm happy to go ahead with it. Since you changed the public interfaces anyway, I think we can't keep backward compatibility and will need to bump major version, unless you see some way? |
Thanks for taking a look, its great to here you are interested in the concept. I will try and find time to explore the options backwards compat and finishing touches on the PR over the next few weeks. |
pkg/sql/pgx/pgx_adapter.go
Outdated
| func (t Tx) Rollback() error { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | ||
| defer cancel() | ||
|
|
||
| return t.Tx.Rollback(ctx) | ||
| } | ||
|
|
||
| func (t Tx) Commit() error { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | ||
| defer cancel() | ||
|
|
||
| return t.Tx.Commit(ctx) | ||
| } |
There was a problem hiding this comment.
Because the stdSQL interface this interface uses doesn't take a context, it looks like I decided to create a context and add a timeout here.
I don't want to refactor too much to make context available here in this PR.
- I could just use a background context with no timeout
- I could make the timeout configurable when creating this adapter
- I could keep a hardcoded default timeout as it is currently (I don't like this as much)
I am opening this draft PR to see if you are interested in this direction for the library (trimming down the interfaces to only what is used). If so I can put the effort in to get it up to your required standards and resolve the final issues. If not I can close it and keep my fork.
Thanks for all the great work you have done on the lib so far!
I want to use watermill to fulfil the outbox pattern (persisting events that will be asynchronously broadcast as part of the main persistence transaction) but my company mainly uses the pgx library.
This change makes this library more flexible to allow for the use of the pgx library. I can keep the pgx adapter itself elsewhere, outside of this repo if you would like.
Still to do: