Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Implement total driver system + detach River from
pgx/v5
Previously, we implement a basic driver interface in River such that no
exported River APIs depended on `pgx/v5` directly, with the idea being
that although only one database driver was actually supported, it'd give
us the leeway we needed to fully implement a multi-driver system later.
Under the covers, the drivers were a hack, and `GetDBPool` just returned
the wrapped pgx database pool back to the client and other packages that
wanted to access it.
This change takes the initial driver push and carries it all the way.
The driver API becomes fully fleshed out with all database operations
River needs to operate, and the top-level River module drops internal
use of `pgx/v5` completely (it's still used in tests, but no non-test
packages).
If `pgx/v6` were to be released tomorrow, this would allow us to add
support for it quite easily. It also allows us to theoretically fully
implement the `databasesql` driver, or to implement drivers for other
databases like MySQL and SQLite (it'd be work to get that done, but
the driver API is generic enough to make it possible).
Notably, the `dbadapter` package goes away because the River driver API
becomes a very similar concept to it, with interface functions for major
operations. A difference is that given we'd like to implement multiple
drivers, the driver code should stay as lean as possible so that we
don't have to reimplement it over and over again, so any higher level
logic that could be extracted from `StandardAdapter` was, including:
* About half of the list logic was already in `dblist`, but we move the
other half too so that all list logic is now packaged together.
* Uniqueness logic moves to a new `dbunique` package.
`dbadapter` tests move into a new module `riverdrivertest` that provides
some helpers that easily allow us to run the full barrage against
`riverpgxv5`, but also any new drivers that we might add in the future.
(`riverdatabasesql` is also tested, but uses a different helper that
only checks its support for migrations.)
Possibly the trickiest incision was moving listen/notify logic over to
the driver, which picks up a new `GetListener()` function that returns
this `Listener` interface:
// Listner listens for notifications. In Postgres, this is a database connection
// where `LISTEN` has been run.
//
// API is not stable. DO NOT IMPLEMENT.
type Listener interface {
Close(ctx context.Context) error
Connect(ctx context.Context) error
Listen(ctx context.Context, topic string) error
Ping(ctx context.Context) error
Unlisten(ctx context.Context, topic string) error
WaitForNotification(ctx context.Context) (*Notification, error)
}
This is backed by a connection pool and `LISTEN` for `riverpgxv5`, but
the idea is that for databases that don't support listen/notify, it's
generic enough that it could be reimplemented as something else under
the hood, like say an unlogged table of some sort.
`river/riverdriver` becomes its own Go module. Previously, we'd gotten
away without referencing it from driver implementations, but the
implementations now reference it for parameter types and some interfaces
and return values. Like most other modules, modules that need to
reference `riverdriver` use `replace` in their `go.mod`s, so I don't
expect the extra module to add any extra headache to our releases.
Discerning readers may notice that driver implementations reference some
types in `riverdriver` and other types in `rivertypes`. I'm defining
which is which as: `riverdriver` should contain anything unstable that
we're allowed to change and which a user shouldn't be accessing anyway
(e.g. say function parameter structs). `rivertypes` is for specific
constructs that external River users will need to reference like
`JobRow`.
Lastly, I'll say that this isn't final. I've refactored a lot of tests
to ensure that this patch doesn't have any major outstanding TODOs and
doesn't leave the code any worse that when it started, but there are a
lot of follow up improvements that I can potentially make, and expect
to.- Loading branch information
