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

Minor 1.11.0 break 1.10.4 on drop_constraint() takes 2 positional arguments but 3 were given #1245

Closed
YonatanKiron opened this issue May 16, 2023 · 3 comments

Comments

@YonatanKiron
Copy link

Describe the bug

I'm using airflow, and it's depends on alembic<2.0,>=1.5.1 (Airflow 2.3.4)
Since yesterday, we are experiencing an issue that:

  File "/home/jenkins/agent/workspace/anager_airflow_run_tests_PR-1537/.tox/py/lib/python3.8/site-packages/airflow/migrations/versions/0093_2_2_0_taskinstance_keyed_to_dagrun.py", line 157, in upgrade
    batch_op.drop_constraint('task_reschedule_dag_task_date_fkey', 'foreignkey')
TypeError: drop_constraint() takes 2 positional arguments but 3 were given

It seems like, from some reason, that alembic 1.11.0 breaks 1.10.4

Expected behavior

  • In case of breaking change, bump major not a minor 🤷🏼‍♂️
  • Not breaking minor version
  • Test to would have catch that scenario

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.
Install airflow 2.3.4
run airflow db init

# Insert code here

Error

  File "/home/jenkins/agent/workspace/anager_airflow_run_tests_PR-1537/.tox/py/lib/python3.8/site-packages/airflow/migrations/versions/0093_2_2_0_taskinstance_keyed_to_dagrun.py", line 157, in upgrade
    batch_op.drop_constraint('task_reschedule_dag_task_date_fkey', 'foreignkey')
TypeError: drop_constraint() takes 2 positional arguments but 3 were given

Versions.

  • OS:
  • Python: 3.8.11
  • Alembic: 1.11.0
  • SQLAlchemy: 0.41.1
  • Database: postgresql/sqlite
  • DBAPI:

Additional context

Currently, we strict our alembic version in out requirements.txt just want to raise this flag 🎏

Have a nice day!

@YonatanKiron YonatanKiron added the requires triage New issue that requires categorization label May 16, 2023
@zzzeek
Copy link
Member

zzzeek commented May 16, 2023

We will check version for where autogenerate was doing this and add to our fixes, but just so you know alembic and sqlalchemy don't do pure semver and 1.11 is a "major release". we do plan to deprecate these calling styles in 1.12 or so

@zzzeek
Copy link
Member

zzzeek commented May 16, 2023

Alembic's autogenerate always generated this argument as a keyword argument, but it looks like we have one example in the docs that uses this positionally, so I will assume airflow devs worked from that example here.

Airflow will have to update this migration at some point though

@sqla-tester
Copy link
Collaborator

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

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

@zzzeek zzzeek added op directives and removed requires triage New issue that requires categorization labels May 16, 2023
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

3 participants