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

TypeError: drop_index() takes 2 positional arguments but 3 were given #1243

Closed
bobbypriam opened this issue May 16, 2023 · 6 comments
Closed

Comments

@bobbypriam
Copy link

Describe the bug
Upgrading from 1.10.4 to 1.11.0 breaks existing migration, possibly due to this change: #1130

Expected behavior
Existing migration still works, or a deprecation warning with clear migration path is given.

To Reproduce
Please try to provide a Minimal, Complete, and Verifiable example, with the migration script and/or the SQLAlchemy tables or models involved.
See also Reporting Bugs on the website.

"""dummy revision

Revision ID: cc20a95c17c5
Revises:
Create Date: 2023-05-16 13:41:28.641085

"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = 'cc20a95c17c5'
down_revision = None
branch_labels = None
depends_on = None


def upgrade() -> None:
    # This worked on 1.10.4, but broke on 1.11.0
    op.drop_index("dummy_index_name", "dummy_table_name")
    pass


def downgrade() -> None:
    pass

Error

➵  alembic upgrade head
INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> cc20a95c17c5, dummy revision
Traceback (most recent call last):
  File "/Users/<user>/Work/alembic-test/.venv/bin/alembic", line 8, in <module>
    sys.exit(main())
  File "/Users/<user>/Work/alembic-test/.venv/lib/python3.10/site-packages/alembic/config.py", line 617, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/Users/<user>/Work/alembic-test/.venv/lib/python3.10/site-packages/alembic/config.py", line 611, in main
    self.run_cmd(cfg, options)
  File "/Users/<user>/Work/alembic-test/.venv/lib/python3.10/site-packages/alembic/config.py", line 588, in run_cmd
    fn(
  File "/Users/<user>/Work/alembic-test/.venv/lib/python3.10/site-packages/alembic/command.py", line 385, in upgrade
    script.run_env()
  File "/Users/<user>/Work/alembic-test/.venv/lib/python3.10/site-packages/alembic/script/base.py", line 582, in run_env
    util.load_python_file(self.dir, "env.py")
  File "/Users/<user>/Work/alembic-test/.venv/lib/python3.10/site-packages/alembic/util/pyfiles.py", line 94, in load_python_file
    module = load_module_py(module_id, path)
  File "/Users/<user>/Work/alembic-test/.venv/lib/python3.10/site-packages/alembic/util/pyfiles.py", line 110, in load_module_py
    spec.loader.exec_module(module)  # type: ignore
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/Users/<user>/Work/alembic-test/alembic/env.py", line 78, in <module>
    run_migrations_online()
  File "/Users/<user>/Work/alembic-test/alembic/env.py", line 72, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/Users/<user>/Work/alembic-test/.venv/lib/python3.10/site-packages/alembic/runtime/environment.py", line 928, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/Users/<user>/Work/alembic-test/.venv/lib/python3.10/site-packages/alembic/runtime/migration.py", line 628, in run_migrations
    step.migration_fn(**kw)
  File "/Users/<user>/Work/alembic-test/alembic/versions/cc20a95c17c5_dummy_revision.py", line 20, in upgrade
    op.drop_index("dummy_index_name", "dummy_table_name")
  File "<string>", line 8, in drop_index
TypeError: drop_index() takes 2 positional arguments but 3 were given

Versions.

  • OS: macOS 13.3.1 (22E261) - also get same error in Docker
  • Python: Python 3.10.10
  • Alembic: alembic==1.11.0
  • SQLAlchemy: SQLAlchemy==2.0.13
  • Database: MySQL
  • DBAPI: PyMySQL==1.0.3

Additional context

Have a nice day!

@bobbypriam bobbypriam added the requires triage New issue that requires categorization label May 16, 2023
@Vishal2696
Copy link

+1. Same here. The upgrade is breaking. Same error too.

@zzzeek
Copy link
Member

zzzeek commented May 16, 2023

ah i was almost going to change that one too.

@zzzeek
Copy link
Member

zzzeek commented May 16, 2023

Can I just confirm that the migration given is fairly old, op.drop_index("dummy_index_name", "dummy_table_name") should not be the format that we output now. I would need to check when we changed this.

@zzzeek zzzeek added autogenerate - rendering and removed requires triage New issue that requires categorization labels May 16, 2023
@zzzeek
Copy link
Member

zzzeek commented May 16, 2023

hm, OK so, the correct form of this migration:

def upgrade() -> None:
    # This worked on 1.10.4, but broke on 1.11.0
    op.drop_index("dummy_index_name", "dummy_table_name")
    pass

should be this:

def upgrade() -> None:
    op.drop_index("dummy_index_name", table_name="dummy_table_name")
    pass

It looks like the previous syntax was changed in this commit: 20c0806. that was in 2014.

kind of a tossup if we just please ask users to upgrade very old migrations or not, but I'll restore it now anyway.

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the main branch:

restore drop_index.table_name as positional https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4618

MoritzWeber0 added a commit to DSD-DBS/capella-collab-manager that referenced this issue May 16, 2023
The alembic update contained breaking changes.
We should manually update the library in the future.
sqlalchemy/alembic#1243
@bobbypriam
Copy link
Author

Yeah, this is from an old migration. Thank you so much for looking into this.

nsoranzo added a commit to nsoranzo/galaxy that referenced this issue May 17, 2023
Alembic 1.11.0 made the `table_name` argument of `op.drop_index()`
keyword-only, causing the following failure in the
test_galaxy_packages build (where alembic is not pinned):

```
galaxy/model/migrations/util.py:30: error: Too many positional arguments for
"drop_index"  [misc]
            op.drop_index(index_name, table_name)
            ^
```

xref: sqlalchemy/alembic#1243

Also drop the unused `columns` parameter from our wrapper method,
as done already in the dev branch.
MoritzWeber0 added a commit to DSD-DBS/capella-collab-manager that referenced this issue May 24, 2023
The alembic update contained breaking changes.
We should manually update the library in the future.
sqlalchemy/alembic#1243
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants