Skip to content

fix: broken alembic import with alembic 1.15. Solves issue #368 - #369

Open
thomas-dufour wants to merge 1 commit into
xzkostyan:masterfrom
thomas-dufour:fix_368
Open

fix: broken alembic import with alembic 1.15. Solves issue #368#369
thomas-dufour wants to merge 1 commit into
xzkostyan:masterfrom
thomas-dufour:fix_368

Conversation

@thomas-dufour

Copy link
Copy Markdown

Checklist:

  • test locally that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Ensure PR doesn't contain untouched code reformatting: spaces, etc.
  • Run flake8 and fix issues.
  • Run pytest no tests failed. See https://clickhouse-sqlalchemy.readthedocs.io/en/latest/development.html.

@cemsozens

Copy link
Copy Markdown

@xzkostyan Could you take a look at this PR? It will be really helpful to merge this.

@sk-

sk- commented Jun 2, 2025

Copy link
Copy Markdown

@xzkostyan is there any chance this PR could be reviewed and released.

The PR looks good, as the changes are the same that were done in the original alembic's PR.

@thomas-dufour

Copy link
Copy Markdown
Author

@PabloReszczynski thx for the approval. What about merging this PR (I don't know the process in this repo)? Is there anything I need to do?

stankudrow pushed a commit to stankudrow/clickhouse-sqlalchemy that referenced this pull request Nov 1, 2025
Comment on lines +40 to +43
elif alembic_version >= (1, 11, 0):
return sqla_compat._reflect_table(inspector, table)
else:
return _alembic_reflect_table(inspector, table, None)
return sqla_compat._reflect_table(inspector, table, None)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
elif alembic_version >= (1, 11, 0):
return sqla_compat._reflect_table(inspector, table)
else:
return _alembic_reflect_table(inspector, table, None)
return sqla_compat._reflect_table(inspector, table, None)
if alembic_version >= (1, 11, 0):
return sqla_compat._reflect_table(inspector, table)
return sqla_compat._reflect_table(inspector, table, None)

Just if is enough in lieu of an elif and then else clause because of immediate return statements.

@stankudrow

Copy link
Copy Markdown

@thomas-dufour , @xzkostyan , hello. Is this PR finished and if so, when can it be merged?

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.

Broken import due to Alembic removed _reflect_table

6 participants