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.

Sign up for GitHub

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

Jump to bottom

feat(duckdb): add support for read_mysql #8656

Merged
merged 1 commit into from
Mar 20, 2024
Merged

feat(duckdb): add support for read_mysql #8656

merged 1 commit into from
Mar 20, 2024

Conversation

ncclementi
Copy link
Contributor

@ncclementi
Copy link
Contributor Author

ncclementi commented Mar 14, 2024 •
edited
Loading

Well looks like we triggered the "localhost" issue in CI too see failure here

Locally I noticed that if I use 127.0.0.1 instead of localhost I don't have this problem. But this makes no sense to me.

E    pymysql.err.OperationalError: (2003, "Can't connect to MySQL server
 on 'localhost' ([Errno 111] Connection refused)")

../../../.cache/pypoetry/virtualenvs/ibis-framework-2c2VoWo5-py3.9/lib/python3.9/site-packages/pymysql 
connections.py:711: OperationalError

@cpcloud cpcloud mentioned this pull request Mar 15, 2024
Closed
@cpcloud
Copy link
Member

cpcloud commented Mar 15, 2024

I opened duckdb/duckdb-mysql#44 to track the localhost issue.

@ncclementi ncclementi marked this pull request as ready for review March 15, 2024 22:02
cpcloud
cpcloud approved these changes Mar 19, 2024
View reviewed changes
Copy link
Member

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

ibis/backends/duckdb/__init__.py Show resolved Hide resolved
ibis/backends/duckdb/tests/test_register.py Show resolved Hide resolved
@ncclementi
Copy link
Contributor Author

So, this was fixed duckdb/duckdb-mysql#44, but we can't use it unless we install the extension from the nightlies, so we'll have to wait a bit.

@cpcloud cpcloud added this to the 9.0 milestone Mar 20, 2024
@cpcloud cpcloud added feature Features or general enhancements duckdb The DuckDB backend labels Mar 20, 2024
@cpcloud cpcloud merged commit 4ea4a1d into ibis-project:main Mar 20, 2024
84 checks passed
This was referenced Mar 20, 2024
Closed
Closed
@ncclementi ncclementi mentioned this pull request Mar 29, 2024
Merged
gforsyth pushed a commit that referenced this pull request Apr 2, 2024
@ncclementi
chore: make arguments required-keyword for read_postgres and read_sql…
a5de9ed 
…ite (#8829)

In #8656 we made the args for
`read_mysql` required-keyword.

This PR, makes the args for `read_postgres` and `read_sqlite` to be
required-keyword to be consistent.
It also fixes a broken test that runs only locally, `test_read_postgres`
(thanks @gforsyth for the patch suggestion).

- Closes #8712
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpcloud cpcloud cpcloud approved these changes

Assignees
No one assigned
Labels
duckdb The DuckDB backend feature Features or general enhancements
Projects
None yet
Milestone
9.0
Development

Successfully merging this pull request may close these issues.

feat(duckdb): add support for reading tables from MySQL
2 participants
@ncclementi @cpcloud