-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat(python): Support use of SQLAlchemy "Connectable" in write_database
#17470
feat(python): Support use of SQLAlchemy "Connectable" in write_database
#17470
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17470 +/- ##
==========================================
- Coverage 80.49% 80.49% -0.01%
==========================================
Files 1483 1483
Lines 195010 195013 +3
Branches 2781 2782 +1
==========================================
- Hits 156972 156969 -3
- Misses 37527 37532 +5
- Partials 511 512 +1 ☔ View full report in Codecov by Sentry. |
write_database
I will double-check tonight (on mobile at the moment), but from what I can see this looks like a good addition; thanks! |
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.
Let's version-gate the tests, as they don't work on earlier versions of SQLAlchemy. (I will typically run DB tests with both versions locally, as not everyone has moved to v2.0 yet). The two suggested changes also require the additional import from polars._utils.various import parse_version
at the top of the test module.
Otherwise, looks good to me 👍
c364d73
to
7c5ae5e
Compare
Since it's entirely up to the user to control the transaction, I decided it was okay to just modify the test. |
This is unrelated to this pr, but when I lowered the versions of
I know that |
Even better; we'll have coverage for both versions ;) |
pandas
supportssa.engine.Connectable
.Passing
sa.engine.Connectable
allows the user to control the transaction.