-
Notifications
You must be signed in to change notification settings - Fork 590
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
fix(duckdb): allow connection to motherduck via ibis.connect #8357
Conversation
|
Not adding a test because I don't want to monkeypatch a bunch of stuff, but: |
Special case if the database path starts with the motherduck prefix
| Path(*parts).absolute() if parts and parts != [":memory:"] else ":memory:" | ||
| ) | ||
| database = Path(*parts) if parts and parts != [":memory:"] else ":memory:" | ||
| if (strdatabase := str(database)).startswith("md:") or strdatabase.startswith( |
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.
Should we set up a secret to allow testing this in CI?
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 could do. I don't think they rotate the tokens particularly often and we've already broken it once. I can put in my credentials -- I don't use motherduck except for testing and I'll confirm that I don't have any payment methods set up in there.
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.
last I checked you can't rotate your key :) have to request it be done on their end
|
tested this and it works for me! |
|
I can add a test in a followup, but what's the best way to go about that? |
|
That seems reasonable to me. This is what we do for Snowflake. |
…oject#8357) ## Description of changes Our `_from_url` was a little aggressive in calling `Path().absolute()` on targets. Since we already handle that in `duckdb.do_connect` we can use `urlparse` to rip out the appropriate bits and forward them. ## Issues closed Fixes ibis-project#8355
Description of changes
Our
_from_urlwas a little aggressive in callingPath().absolute()on targets. Since we already handle that in
duckdb.do_connectwe canuse
urlparseto rip out the appropriate bits and forward them.Issues closed
Fixes #8355