From dc5b1fd54b3a3fa36eab46040bd544cdd83480db Mon Sep 17 00:00:00 2001 From: Brandur Date: Thu, 9 Nov 2023 17:47:57 -0800 Subject: [PATCH] Make `riverdriver/riverpgxv5` its own submodule Here, make `riverdriver/riverpgxv5` its own submodule. This does nothing for the time being since the top-level River package is entirely dependent on `riverpgxv5` as the only driver (and its used in examples and tests), but the general idea is that in the future each driver could bring in its own dependencies, and applications using River wouldn't have to pull in every 3rd party database package that River supports. The only problem is that there's a tradeoff: the way I had `riverpgxv5` before is that it referenced `riverdriver.Driver` and made use of an `internal.Sentinel` value from the top-level package to guarantee no one could implement the driver interface externally. That no longer works because it introduce a dependency cycle: `river` needs `riverpgxv5` and `riverpgxv5` needs `river`. So I can remove the top-level types referenced in `riverpgxv5` so the dependency only goes one way, but we lose `internal.Sentinel`, and can no longer guarantee that `Driver` isn't implemented. So put simply: `riverpgxv5` gets its own module, but we move to only a soft guarantee that external users don't implement `Driver` (we spell it out in the docs, but there's nothing technical preventing people from doing it). Overall: I think it's probably worth it. It's pretty clear `Driver` isn't meant for external implementation, there's no advantage to doing so, and the interface is already a little leaky because users could technically call functions on the interface already like `GetDBPool()` and break themselves as those disappear in the future. --- .github/workflows/ci.yml | 2 ++ go.mod | 7 +++-- go.sum | 10 ++++--- internal/sentinel.go | 6 ---- riverdriver/river_driver_interface.go | 7 ----- riverdriver/river_driver_interface_test.go | 10 +++++++ riverdriver/riverpgxv5/go.mod | 14 ++++++++++ riverdriver/riverpgxv5/go.sum | 28 +++++++++++++++++++ riverdriver/riverpgxv5/river_pgx_v5_driver.go | 10 ++----- 9 files changed, 67 insertions(+), 27 deletions(-) delete mode 100644 internal/sentinel.go create mode 100644 riverdriver/river_driver_interface_test.go create mode 100644 riverdriver/riverpgxv5/go.mod create mode 100644 riverdriver/riverpgxv5/go.sum diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 447e3877..257037e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -127,6 +127,8 @@ jobs: # Optional: show only new issues if it's a pull request. The default value is `false`. only-new-issues: true + skip-cache: true + version: v1.54.2 producer_sample: diff --git a/go.mod b/go.mod index 3454986d..ca72c7c0 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/riverqueue/river -go 1.21 +go 1.21.0 require ( github.com/jackc/pgerrcode v0.0.0-20220416144525-469b46aa5efa @@ -22,9 +22,10 @@ require ( github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect github.com/kr/text v0.2.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/riverqueue/river/riverdriver/riverpgxv5 v0.0.0-20231110014757-1a7176abdf3c // indirect github.com/rogpeppe/go-internal v1.10.0 // indirect github.com/spf13/pflag v1.0.5 // indirect - golang.org/x/crypto v0.9.0 // indirect - golang.org/x/text v0.9.0 // indirect + golang.org/x/crypto v0.15.0 // indirect + golang.org/x/text v0.14.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index ae6df891..8c935420 100644 --- a/go.sum +++ b/go.sum @@ -24,6 +24,8 @@ github.com/oklog/ulid/v2 v2.1.0/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNs github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/riverqueue/river/riverdriver/riverpgxv5 v0.0.0-20231110014757-1a7176abdf3c h1:UXvKR9qb10Rp/CQ29I5VqnvnGdATjS5MiGv03KUMDHo= +github.com/riverqueue/river/riverdriver/riverpgxv5 v0.0.0-20231110014757-1a7176abdf3c/go.mod h1:Ocq2VuaKmczYSFUpbxr+o3l2hux0gjdg7qsfcIaNrjw= github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= @@ -40,14 +42,14 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= -golang.org/x/crypto v0.9.0 h1:LF6fAI+IutBocDJ2OT0Q1g8plpYljMZ4+lty+dsqw3g= -golang.org/x/crypto v0.9.0/go.mod h1:yrmDGqONDYtNj3tH8X9dzUun2m2lzPa9ngI6/RUPGR0= +golang.org/x/crypto v0.15.0 h1:frVn1TEaCEaZcn3Tmd7Y2b5KKPaZ+I32Q2OA3kYp5TA= +golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72g= golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc= golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/sync v0.5.0 h1:60k92dhOjHxJkrqnwsfl8KuaHbn/5dl0lUPUklKo3qE= golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/text v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE= -golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= +golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= diff --git a/internal/sentinel.go b/internal/sentinel.go deleted file mode 100644 index f3d6db36..00000000 --- a/internal/sentinel.go +++ /dev/null @@ -1,6 +0,0 @@ -package internal - -// Sentinel provides a type that's internal to River (because this package is -// internal to River). Its purpose to prevent public code from implementing -// interfaces that shouldn't be user-implemented. -type Sentinel struct{} diff --git a/riverdriver/river_driver_interface.go b/riverdriver/river_driver_interface.go index 79f79d55..c47c08aa 100644 --- a/riverdriver/river_driver_interface.go +++ b/riverdriver/river_driver_interface.go @@ -15,8 +15,6 @@ package riverdriver import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" - - "github.com/riverqueue/river/internal" ) // Driver provides a database driver for use with river.Client. @@ -36,11 +34,6 @@ type Driver[TTx any] interface { // where multiple drivers are supported and is subject to change. GetDBPool() *pgxpool.Pool - // GetSentinel returns a sentinal value of a type that's internal to River. - // Its purpose is to prevent public code from implementing this interface - // because it's not meant to be used externally at this time. - GetSentinel() internal.Sentinel - // UnwrapTx turns a generically typed transaction into a pgx.Tx for use with // internal infrastructure. This doesn't make sense in a world where // multiple drivers are supported and is subject to change. diff --git a/riverdriver/river_driver_interface_test.go b/riverdriver/river_driver_interface_test.go new file mode 100644 index 00000000..221ace9d --- /dev/null +++ b/riverdriver/river_driver_interface_test.go @@ -0,0 +1,10 @@ +package riverdriver + +import ( + "github.com/jackc/pgx/v5" + + "github.com/riverqueue/river/riverdriver/riverpgxv5" +) + +// Verify interface compliance. +var _ Driver[pgx.Tx] = &riverpgxv5.Driver{} diff --git a/riverdriver/riverpgxv5/go.mod b/riverdriver/riverpgxv5/go.mod new file mode 100644 index 00000000..e791133e --- /dev/null +++ b/riverdriver/riverpgxv5/go.mod @@ -0,0 +1,14 @@ +module github.com/riverqueue/river/riverdriver/riverpgxv5 + +go 1.21.0 + +require github.com/jackc/pgx/v5 v5.5.0 + +require ( + github.com/jackc/pgpassfile v1.0.0 // indirect + github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect + github.com/jackc/puddle/v2 v2.2.1 // indirect + golang.org/x/crypto v0.15.0 // indirect + golang.org/x/sync v0.5.0 // indirect + golang.org/x/text v0.14.0 // indirect +) diff --git a/riverdriver/riverpgxv5/go.sum b/riverdriver/riverpgxv5/go.sum new file mode 100644 index 00000000..b9c08498 --- /dev/null +++ b/riverdriver/riverpgxv5/go.sum @@ -0,0 +1,28 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= +github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= +github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a h1:bbPeKD0xmW/Y25WS6cokEszi5g+S0QxI/d45PkRi7Nk= +github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM= +github.com/jackc/pgx/v5 v5.5.0 h1:NxstgwndsTRy7eq9/kqYc/BZh5w2hHJV86wjvO+1xPw= +github.com/jackc/pgx/v5 v5.5.0/go.mod h1:Ig06C2Vu0t5qXC60W8sqIthScaEnFvojjj9dSljmHRA= +github.com/jackc/puddle/v2 v2.2.1 h1:RhxXJtFG022u4ibrCSMSiu5aOq1i77R3OHKNJj77OAk= +github.com/jackc/puddle/v2 v2.2.1/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +golang.org/x/crypto v0.15.0 h1:frVn1TEaCEaZcn3Tmd7Y2b5KKPaZ+I32Q2OA3kYp5TA= +golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72g= +golang.org/x/sync v0.5.0 h1:60k92dhOjHxJkrqnwsfl8KuaHbn/5dl0lUPUklKo3qE= +golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/riverdriver/riverpgxv5/river_pgx_v5_driver.go b/riverdriver/riverpgxv5/river_pgx_v5_driver.go index 7338677a..fac0dbd2 100644 --- a/riverdriver/riverpgxv5/river_pgx_v5_driver.go +++ b/riverdriver/riverpgxv5/river_pgx_v5_driver.go @@ -8,9 +8,6 @@ package riverpgxv5 import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" - - "github.com/riverqueue/river/internal" - "github.com/riverqueue/river/riverdriver" ) // Driver is an implementation of riverdriver.Driver for Pgx v5. @@ -25,10 +22,9 @@ type Driver struct { // // The pool must not be closed while the associated client is running (not until // graceful shutdown has completed). -func New(dbPool *pgxpool.Pool) riverdriver.Driver[pgx.Tx] { +func New(dbPool *pgxpool.Pool) *Driver { return &Driver{dbPool: dbPool} } -func (d *Driver) GetDBPool() *pgxpool.Pool { return d.dbPool } -func (d *Driver) GetSentinel() internal.Sentinel { return internal.Sentinel{} } -func (d *Driver) UnwrapTx(tx pgx.Tx) pgx.Tx { return tx } +func (d *Driver) GetDBPool() *pgxpool.Pool { return d.dbPool } +func (d *Driver) UnwrapTx(tx pgx.Tx) pgx.Tx { return tx }