Add ability to configure alembic_version table in DialectImpl #1563
Conversation
|
ok, nice job with the test. I can clean that up to make it use a unique name |
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 0dc0bda of this pull request into gerrit so we can run tests and reviews and stuff
|
New Gerrit review created for change 0dc0bda: https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556 |
|
@zzzeek |
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision e98a156 of this pull request into gerrit so we can run tests and reviews and stuff
|
Patchset e98a156 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556 |
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of CaselIT to try to get revision 3ff240f of this pull request into gerrit so we can run tests and reviews and stuff
|
Patchset 3ff240f added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556 |
|
@zzzeek My idea is to add condition on |
|
There is a select you can use in the util sqla_compat module |
|
@CaselIT |
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of CaselIT to try to get revision 14539e4 of this pull request into gerrit so we can run tests and reviews and stuff
|
Patchset 14539e4 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556 |
sure! thanks for the effort! |
|
oh I guess since the last CI mypy was updated and you had to fix also mypy to make the ci pass. Thanks and feel free to just tell us to complete this. making the CI happy can be tedious at times |
|
@CaselIT One more test please. |
there should be pre-commit configured that can run all tests |
pre-commit is not triggering tests and mypy :( |
|
@maver1ck can you rebase / merge main? |
maver1ck
force-pushed
the
alembic_starrocks
branch
from
4a63b86 to
120dda7
Compare
maver1ck
force-pushed
the
alembic_starrocks
branch
from
120dda7 to
e70fdc8
Compare
Rebased. |
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of CaselIT to try to get revision e70fdc8 of this pull request into gerrit so we can run tests and reviews and stuff
|
Patchset e70fdc8 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556 |
There was a problem hiding this comment.
Federico Caselli (CaselIT) wrote:
I've left a few comment, nothing major though.
Great work!
View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556
- /COMMIT_MSG (line 18): we should add a line that says
Fixes: #1560
- docs/build/unreleased/vers.rst (line 2): missing ticket here. also the file name could be named as the ticked.
Issue is 1560
|
@CaselIT |
There was a problem hiding this comment.
Michael Bayer (zzzeek) wrote:
code review left on gerrit
View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556
- docs/build/unreleased/vers.rst (line 2): i didnt realize we had a ticket for this
(that's how out of it I am)
| """create the Table object for the version_table. | ||
|
|
||
| Provided as part of impl so that third party dialects can override | ||
| this. |
There was a problem hiding this comment.
Michael Bayer (zzzeek) wrote:
im not convinced we need to bump here, but I guess numbers are free
View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556
There was a problem hiding this comment.
Michael Bayer (zzzeek) wrote:
Done
View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556
There was a problem hiding this comment.
Federico Caselli (CaselIT) wrote:
code review left on gerrit
View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556
- docs/build/unreleased/vers.rst (line 2): I repurposed the one opened for the initial PR. Seemed like it fit
There was a problem hiding this comment.
Michael Bayer (zzzeek) wrote:
code review left on gerrit
View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556
- /COMMIT_MSG (line 18): Done
- docs/build/unreleased/vers.rst (line 2): Done
| @@ -136,6 +139,37 @@ def static_output(self, text: str) -> None: | |||
| self.output_buffer.write(text + "\n\n") | |||
| self.output_buffer.flush() | |||
|
|
|||
| def version_table_impl( | |||
| self, | |||
There was a problem hiding this comment.
Michael Bayer (zzzeek) wrote:
Done
View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556
|
Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5556 has been merged. Congratulations! :) |
Description
This is followup PR to discusion about custom alembic_version table definition
Reference: #1562
Fixes: #1560
Checklist
This pull request is:
must include a complete example of the issue. one line code fixes without an
issue and demonstration will not be accepted.
Fixes: #<issue number>in the commit messageinclude a complete example of how the feature would look.
Fixes: #<issue number>in the commit messageHave a nice day!