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

Improve re-writer implementation, fixing chain and adding support to callables #1337

Closed
CaselIT opened this issue Oct 24, 2023 · 1 comment
Closed

Comments

@CaselIT
Copy link
Member

CaselIT commented Oct 24, 2023

ive just read the source code for chain() and it's a bit of a disaster. because if i do this:

w = writer1.chain(writer2).chain(writer3)

writer2 is lost. it's doing the chaining wrong. my high school comp sci teacher would be unimpressed

it works for your use case right now because it only handles exactly two writers. but if we fix it to correctly implement chaining then it would not work for your case in the general sense.

we can fix both issues at once by 1. fixing the chaining so that if self._chain is non-None we traverse the list through to the end for the linked list append, and 2. looking at the type of object sent and if it's just a callable, wrapping it in a helper object CallableRewriter that uses that callable and also supports the rest of the Rewriter protocol including self._chain.

Originally posted by @zzzeek in #1332 (reply in thread)

zrotceh added a commit to zrotceh/alembic that referenced this issue Nov 29, 2023
Fixes sqlalchemy#1337 by:
* Fix the chaining of more than two rewriters
* Wrap a callable so that it could be chained
@sqla-tester
Copy link
Collaborator

l-hedgehog has proposed a fix for this issue in the main branch:

Improve Rewriter implementation https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4990

zrotceh added a commit to zrotceh/alembic that referenced this issue Dec 11, 2023
Fixes sqlalchemy#1337 by:
* Fix the chaining of more than two rewriters
* Accept the chaining of a callable as well
zrotceh added a commit to zrotceh/alembic that referenced this issue Dec 11, 2023
Fixes sqlalchemy#1337 by:
* Fix the chaining of more than two rewriters
* Accept the chaining of a callable as well
zrotceh added a commit to zrotceh/alembic that referenced this issue Dec 11, 2023
Fixes sqlalchemy#1337 by:
* Fix the chaining of more than two rewriters
* Accept the chaining of a callable as well
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants