Skip to content

Commit 8fbad91

Browse files
authored
Implement total driver system + detach River from pgx/v5 (#212)
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.
1 parent 48ef9ef commit 8fbad91

File tree

95 files changed

+7481
-5132
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

95 files changed

+7481
-5132
lines changed

.github/workflows/ci.yml

+11-1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ jobs:
8585
working-directory: ./riverdriver/riverpgxv5
8686
run: go test -race ./...
8787

88+
- name: Test rivertype
89+
working-directory: ./rivertype
90+
run: go test -race ./...
91+
8892
cli:
8993
runs-on: ubuntu-latest
9094
timeout-minutes: 3
@@ -186,6 +190,12 @@ jobs:
186190
version: ${{ env.GOLANGCI_LINT_VERSION }}
187191
working-directory: ./riverdriver/riverpgxv5
188192

193+
- name: Lint rivertype
194+
uses: golangci/golangci-lint-action@v3
195+
with:
196+
version: ${{ env.GOLANGCI_LINT_VERSION }}
197+
working-directory: ./rivertype
198+
189199
producer_sample:
190200
runs-on: ubuntu-latest
191201
timeout-minutes: 2
@@ -250,7 +260,7 @@ jobs:
250260
- name: Setup sqlc
251261
uses: sqlc-dev/setup-sqlc@v4
252262
with:
253-
sqlc-version: "1.24.0"
263+
sqlc-version: "1.25.0"
254264

255265
- name: Run sqlc diff
256266
run: |

CHANGELOG.md

+9
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- `JobListParams.Kinds()` has been added so that jobs can now be listed by kind. [PR #212](https://github.com/riverqueue/river/pull/212).
13+
14+
### Changed
15+
16+
- The underlying driver system's been entirely revamped so that River's non-test code is now decoupled from `pgx/v5`. This will allow additional drivers to be implemented, although there are no additional ones for now. [PR #212](https://github.com/riverqueue/river/pull/212).
17+
1018
### Fixed
1119

20+
- Fix a problem where `JobListParams.Queues()` didn't filter correctly based on its arguments. [PR #212](https://github.com/riverqueue/river/pull/212).
1221
- Fix a problem in `DebouncedChan` where it would fire on its "out" channel too often when it was being signaled continuousy on its "in" channel. This would have caused work to be fetched more often than intended in busy systems. [PR #222](https://github.com/riverqueue/river/pull/222).
1322

1423
## [0.0.22] - 2024-02-19

Makefile

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ generate: generate/sqlc
44

55
.PHONY: generate/sqlc
66
generate/sqlc:
7-
cd internal/dbsqlc && sqlc generate
87
cd riverdriver/riverdatabasesql/internal/dbsqlc && sqlc generate
98
cd riverdriver/riverpgxv5/internal/dbsqlc && sqlc generate
109

@@ -15,6 +14,7 @@ lint:
1514
cd riverdriver && golangci-lint run --fix
1615
cd riverdriver/riverdatabasesql && golangci-lint run --fix
1716
cd riverdriver/riverpgxv5 && golangci-lint run --fix
17+
cd rivertype && golangci-lint run --fix
1818

1919
.PHONY: test
2020
test:
@@ -23,13 +23,13 @@ test:
2323
cd riverdriver && go test ./...
2424
cd riverdriver/riverdatabasesql && go test ./...
2525
cd riverdriver/riverpgxv5 && go test ./...
26+
cd rivertype && go test ./...
2627

2728
.PHONY: verify
2829
verify:
2930
verify: verify/sqlc
3031

3132
.PHONY: verify/sqlc
3233
verify/sqlc:
33-
cd internal/dbsqlc && sqlc diff
3434
cd riverdriver/riverdatabasesql/internal/dbsqlc && sqlc diff
3535
cd riverdriver/riverpgxv5/internal/dbsqlc && sqlc diff

0 commit comments

Comments
 (0)