Add ability to configure alembic_version table in DialectImpl

sqlalchemy / alembic

A database migrations tool for SQLAlchemy.

MIT License

2.89k stars 247 forks source link

Add ability to configure alembic_version table in DialectImpl #1563

Closed maver1ck closed 2 weeks ago

maver1ck commented 3 weeks ago

Description

This is followup PR to discusion about custom alembic_version table definition Reference: https://github.com/sqlalchemy/alembic/discussions/1562 Fixes: https://github.com/sqlalchemy/alembic/issues/1560

Checklist

This pull request is:

[ ] A documentation / typographical error fix
- Good to go, no issue or tests are needed
[ ] A short code fix
- please include the issue number, and create an issue if none exists, which must include a complete example of the issue. one line code fixes without an issue and demonstration will not be accepted.
- Please include: Fixes: #<issue number> in the commit message
- please include tests. one line code fixes without tests will not be accepted.
[x] A new feature implementation
- please include the issue number, and create an issue if none exists, which must include a complete example of how the feature would look.
- Please include: Fixes: #<issue number> in the commit message
- please include tests.

Have a nice day!

zzzeek commented 3 weeks ago

ok, nice job with the test. I can clean that up to make it use a unique name

sqla-tester commented 3 weeks ago

New Gerrit review created for change 0dc0bda1e8c5182d2d70360e779564a3c44d9097: https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556

maver1ck commented 3 weeks ago

@zzzeek I fixed linting issues and hopefully problem with sqla 1.3. (with_only_column is supported in 1.3)

sqla-tester commented 3 weeks ago

Patchset e98a15658ded31eebc75b23be35b3da5d4c631e7 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556

sqla-tester commented 3 weeks ago

Patchset 3ff240fc43e2c90cd5d2bb35783aa9d0aa602fd7 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556

maver1ck commented 3 weeks ago

@zzzeek I can't find a way of selecting one column that is both compatible with SQLAlchemy 1.3 and (1.4, 2.0). Any ideas ?

My idea is to add condition on sqla_14 version.

CaselIT commented 3 weeks ago

There is a select you can use in the util sqla_compat module

maver1ck commented 3 weeks ago

@CaselIT Fixed. May I ask for another test run ?

sqla-tester commented 3 weeks ago

Patchset 14539e42a63031b2459f0ffd8d969b9768d31f96 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556

CaselIT commented 3 weeks ago

@CaselIT Fixed. May I ask for another test run ?

sure! thanks for the effort!

CaselIT commented 3 weeks ago

oh I guess since the last CI mypy was updated and you had to fix also mypy to make the ci pass. Sorry about that.

Thanks and feel free to just tell us to complete this. making the CI happy can be tedious at times

maver1ck commented 3 weeks ago

@CaselIT No problem at all. I'm really missing bash script that will run all checks and tests :)

One more test please.

CaselIT commented 3 weeks ago

@CaselIT No problem at all. I'm really missing bash script that will run all checks and tests :)

One more test please.

there should be pre-commit configured that can run all tests

maver1ck commented 3 weeks ago

@CaselIT No problem at all. I'm really missing bash script that will run all checks and tests :) One more test please.

there should be pre-commit configured that can run all tests

pre-commit is not triggering tests and mypy :(

maver1ck commented 3 weeks ago

@zzzeek Please also find PR fixing mypy issues. https://github.com/sqlalchemy/alembic/pull/1564

CaselIT commented 3 weeks ago

@maver1ck can you rebase / merge main?

maver1ck commented 3 weeks ago

@maver1ck can you rebase / merge main?

Rebased.

sqla-tester commented 3 weeks ago

Patchset e70fdc8f4e405cabf5099c2100763d7b24da3be8 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556

maver1ck commented 3 weeks ago

@CaselIT I fixed typing issues in the code however I don't know how to handle commit message and vers.rst file

sqla-tester commented 2 weeks ago

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556 has been merged. Congratulations! :)