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

Added support for timescaledb #16

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

gedemagt
Copy link

@gedemagt gedemagt commented Dec 5, 2019

Currently the implementation of postgres fails when trying to recover a TimescaleDB, due to their special hypertables.

This PR is mostly a copy of the postgres backend, but with the important difference that the we drop the extension before we drop the tables:

    cursor.execute('''
            DROP EXTENSION IF EXISTS timescaledb CASCADE;
        ''')

@coveralls
Copy link

coveralls commented Dec 5, 2019

Coverage Status

Coverage decreased (-2.5%) to 91.579% when pulling 3021ded on gedemagt:master into 9ad2b3e on TeamHG-Memex:master.

@mehaase
Copy link
Contributor

mehaase commented Dec 14, 2019

Thank you for the PR @gedemagt, and I apologize for not responding yet. I'll try to review it soon. I don't have any experience with Timescale DB but this basically looks good. Before I merge, I'll need automated tests to cover the new code and updated docs. If you are able to include those in your PR, that will definitely decrease the time to merge.

This PR is mostly a copy of the postgres backend, but with the important difference that the we drop the extension before we drop the tables:

My main question for review is if the difference between timescale and postgres is very small, can you subclass the postgres class and override one or two methods to support timescale?

@gedemagt
Copy link
Author

Good point - I changed it to subclass.

I will have a look at the test as soon as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants