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
Move database connection utils to dbconn #17399
Conversation
Notifying subscribers in CODENOTIFY files for diff f96e136...46ae581.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Did you check a migration just to be sure that nothing broke there?
@ryanslade Not yet, once sqlhooks gets updated I will run integration tests and a migration to check if every is fine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can merge this if we use your fork as a dependency and update later once it's merged upstream.
Codecov Report
@@ Coverage Diff @@
## main #17399 +/- ##
==========================================
+ Coverage 51.87% 51.89% +0.01%
==========================================
Files 1716 1717 +1
Lines 85296 85275 -21
Branches 7647 7634 -13
==========================================
+ Hits 44246 44251 +5
+ Misses 37148 37124 -24
+ Partials 3902 3900 -2
|
There currently are two separate connection helpers, using two different PG drivers.
This unifies database connection helpers and merges them into one single package.
dbconn
is now the only package responsible for connecting to the database and database migrations.dbutil
is now only responsible for generic helpers.Note: This relies on adding support for driver.ConnBeginTx on the sqlhooks package qustavo/sqlhooks#33
Fixes #16586