Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rivermigrate using a single connection instead of a pool #659

Open
roman-vanesyan opened this issue Oct 26, 2024 · 4 comments
Open

rivermigrate using a single connection instead of a pool #659

roman-vanesyan opened this issue Oct 26, 2024 · 4 comments

Comments

@roman-vanesyan
Copy link

Hello!

River is an excellent piece of software, I'm in love with it :)


Recently, I tried to integrate river migration with the migration tooling that uses pgx/v5's pgx.Conn to drive the migration instead of a pgxpool.Pool. Unfortunately, River seems to not supporting it. Previously it supported pgx.Tx, but then it was removed.

Are you open to add support for pgx.Conn/generic single connection?

@bgentry
Copy link
Contributor

bgentry commented Oct 27, 2024

Glad you’re liking it! 😀

This might be a tricky thing to implement. The reason is that the migrator works on a driver, and the pgx driver can currently only be initialized with a pool. We might be able to make it work on a conn with some refactoring, though we’d probably want to make sure people can’t use that single conn driver with a River Client because that would likely have some issues and would be a big foot gun.

I am definitely open to it if we can figure out a way to address those concerns!

@roman-vanesyan
Copy link
Author

roman-vanesyan commented Oct 28, 2024

Ahh, great!

The way I see it, to make the Migrator to work with connections, we can remove reliance on the driver and make Migrate function to accept conn as argument instead.

Connection could be something as simple as dbsqlc.DBTX and sqlc is configured with emit_methods_with_db_argument: true.

WDYT?

@bgentry
Copy link
Contributor

bgentry commented Oct 29, 2024

@roman-vanesyan the migrator needs to continue being able to work with multiple database adapters so I think it's important that it continue operating on an interface like that. It does use a small subset of the driver methods, but certain ones like GetMigrationFS are important in making riverpro work. I think we may need a different approach that works within this design instead 🤔

@kamikazechaser
Copy link
Contributor

kamikazechaser commented Nov 4, 2024

River seems to not supporting it.

riverPgxDriver := riverpgxv5.New(/* *pgxpool.Pool */)
riverMigrator, err := rivermigrate.New(riverPgxDriver, &rivermigrate.Config{})

I'm able to migrate with a pool on v0.12.1 (I don't see any breaking changes yet)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants