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

'BatchOperations' object has no attribute 'rename_table' #1453

Open
stephenfin opened this issue Apr 5, 2024 · 2 comments
Open

'BatchOperations' object has no attribute 'rename_table' #1453

stephenfin opened this issue Apr 5, 2024 · 2 comments
Labels

Comments

@stephenfin
Copy link

stephenfin commented Apr 5, 2024

Describe the bug

After an upgrade to 1.11.x, I now see the following error messages in a migration:

AttributeError: 'BatchOperations' object has no attribute 'rename_table'

The migration in question is doing this:

def upgrade_export_locations_table(connection):
    ...

    with op.batch_alter_table("share_export_locations") as batch_op:
        batch_op.drop_constraint('sel_id_fk', type_='foreignkey')
        batch_op.drop_column('share_id')
        batch_op.rename_table('share_export_locations',
                              'share_instance_export_locations')

Things worked (or at least, didn't fail) on 1.10.4 and before. Now, the ops docs suggest this was never supported, but it worked and therefore we used it.

The fix is rather easy: we can just change the above to:

def upgrade_export_locations_table(connection):
    ...

    with op.batch_alter_table("share_export_locations") as batch_op:
        batch_op.drop_constraint('sel_id_fk', type_='foreignkey')
        batch_op.drop_column('share_id')
    op.rename_table('share_export_locations',
                    'share_instance_export_locations')

...but there's no mention of this change in behavior in the release notes for 1.11. It would be good to add it, even if retrospectively, if this was indeed intentional.

Expected behavior

I'd expect one of the two things to happen:

  • BatchOperations.rename_table is defined as an alias of Operations.rename_table and does the exact same thing
  • There is a note in the release notes for 1.11.0 indicating that this behaviour changed.

(PS: I did try looking at the rel_1_10_4...rel_1_11_0 diff and commit 2aba0ad looks like the most likely candidate for this issue, but I have yet to go much deeper than this)

To Reproduce

See https://github.com/stephenfin/alembic-issue-1453 for a minimal'ish reproducer.

Error

AttributeError: 'BatchOperations' object has no attribute 'rename_table'

Versions.

  • OS: Fedora
  • Python: 3.12
  • Alembic: 1.11.0
  • SQLAlchemy: 1.4.51
  • Database: MySQL
  • DBAPI: mysql

Additional context

Have a nice day!

@stephenfin stephenfin added the requires triage New issue that requires categorization label Apr 5, 2024
@CaselIT
Copy link
Member

CaselIT commented Apr 5, 2024

Hi,

This was caused by the change done as part of #1093 and mentioned in this change note: https://alembic.sqlalchemy.org/en/latest/changelog.html#change-3bdff00f5f667e502dd8506162574a24

I agree that we could note a bit better that the rename_table was removed as a consequence of the change of the class hierarchy in between operation and batch operation.

So I would consider this only a documentation change. @zzzeek do you agree here?

Can you provide a PR with the updated changelog?

@CaselIT CaselIT added bug Something isn't working documentation batch migrations and removed requires triage New issue that requires categorization labels Apr 5, 2024
@zzzeek
Copy link
Member

zzzeek commented Apr 5, 2024

doc change is fine sure

openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Apr 9, 2024
* Update manila from branch 'master'
  to 56db40e344495e162d1255747a274e2641d13447
  - Merge "db: rename_table is not a batch operation"
  - db: rename_table is not a batch operation
    
    This is reported upstream [1] but I suspect the root cause is that we
    were relying on a bug in Alembic: renaming tables doesn't really make
    sense as a batch operation, which by definition works by recreating
    tables with an updated schema (to support SQLite and its lack of full
    'ALTER' support).
    
    [1] sqlalchemy/alembic#1453
    
    Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
    Change-Id: I1da6d117778bbbad64b2df2dfd2f8aeef8a7084c
openstack-mirroring pushed a commit to openstack/manila that referenced this issue Apr 9, 2024
This is reported upstream [1] but I suspect the root cause is that we
were relying on a bug in Alembic: renaming tables doesn't really make
sense as a batch operation, which by definition works by recreating
tables with an updated schema (to support SQLite and its lack of full
'ALTER' support).

[1] sqlalchemy/alembic#1453

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Change-Id: I1da6d117778bbbad64b2df2dfd2f8aeef8a7084c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants