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

timescaledb incompatibility #104

Closed
AntiSol opened this issue Feb 18, 2025 · 2 comments
Closed

timescaledb incompatibility #104

AntiSol opened this issue Feb 18, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@AntiSol
Copy link

AntiSol commented Feb 18, 2025

Hello, I'm the maintainer of a sqlalchemy timescaledb driver. Timescaledb is a postgresql extension which provides some new functionality supported by the timescaledb dialect.

It's a superset of postgres, so postgres enums work just as you'd expect, but when using my timescaledb driver it reports the sql dialect as timescaledb

I've been trying to use alembic-postgresql-enum to manage enums on a timescaledb database, but it gives me a warning saying that it only supports postgresql, but I am using timescaledb.

I had a look at the code and you're checking against a hardcoded postgresql value in alembic_postgresql_enum/compare_dispatch.py and alembic_postgresql_enum/operations/sync_enum_values.py. I found that if I change these checks to something like:

    if autogen_context.dialect.name not in ['postgresql','timescaledb']:
        log.warning(
            f"This library only supports postgresql, but you are using {autogen_context.dialect.name}, skipping"
        )   
        return

Then I am able to use alembic-autogenerate-enum to maintain enums when I am using timescaledb.

I started work on a PR, you can see my branch here, but I wasn't sure whether this is something which is best added as a configuration item, or somewhere else, or about writing tests for this.

@AlexandrovRoman AlexandrovRoman added the enhancement New feature or request label Feb 20, 2025
@AlexandrovRoman
Copy link
Member

Hello. We've looked at the proposed changes. They imply support for postgresql dialects from our side, which we don't have enough resources for. So we have added a flag within #105 to force the library operation to be enabled at your own risk

@AntiSol
Copy link
Author

AntiSol commented Feb 20, 2025

Thanks!
I looked at #105, and it does solve my issue, but I have comments:

  • I feel like e.g skip_dialect_check would have been a better config name than force_dialect_support
  • By skipping the check completely rather than allowing modification of a list of allowed dialects, this runs the risk of running into issues if e.g connected to a non-postgres database, i.e it completely disables the check rather than allowing it to check for sensible values.

Could this be done as a configurable list, and only have postgresql as a default value?
If you have a note saying only the postgresql dialect is supported and timescaledb isn't included by default then that doesn't imply support for other dialects, does it?

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

No branches or pull requests

2 participants