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

CLN: Unify database methods in SQLAlchemy backends #2862

Merged
merged 10 commits into from
Jul 24, 2021
Merged

CLN: Unify database methods in SQLAlchemy backends #2862

merged 10 commits into from
Jul 24, 2021

Conversation

datapythonista
Copy link
Contributor

@datapythonista datapythonista commented Jul 14, 2021

Moving duplicated methods from SQLAlchemy backends to base alchemy backend.

For list_databases and list_schemas, they do the same thing (in different ways) except for postgres. In all other backends list_databases is used, so list_schemas is deprecated everywhere except in postgres.

@datapythonista datapythonista added refactor Issues or PRs related to refactoring the codebase sqlalchemy SQLAlchemy-based backends labels Jul 14, 2021
@jreback jreback added this to the Next release milestone Jul 14, 2021
@jreback
Copy link
Contributor

jreback commented Jul 14, 2021

can you rebase. note I am not sure if these are really tested that well.

@datapythonista
Copy link
Contributor Author

can you rebase. note I am not sure if these are really tested that well.

list_databases seems tested, I didn't check the rest. I'll try to spend some time reviewing the tests of the public API once it's well defined.


def list_schemas(self):
warnings.warn(
'`list_schemas` is deprecated, use `list_databases` instead',
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a test for this. Also we should create an issue with deprecations as a reminder to remove them.

@jreback
Copy link
Contributor

jreback commented Jul 20, 2021

looks fine can you rebase

@datapythonista datapythonista merged commit 5b8590f into ibis-project:master Jul 24, 2021
@cpcloud cpcloud removed this from the Next release milestone Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Issues or PRs related to refactoring the codebase sqlalchemy SQLAlchemy-based backends
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants