[GitHub] [airflow] kaxil commented on a change in pull request #13921: Fix DB Migration for SQLite to upgrade to 2.0

[GitBox <gi...@apache.org>]

kaxil commented on a change in pull request #13921:
URL: https://github.com/apache/airflow/pull/13921#discussion_r566285080



##########
File path: airflow/migrations/versions/cf5dc11e79ad_drop_user_and_chart.py
##########
@@ -47,7 +47,12 @@ def upgrade():  # noqa: D103
 
     if 'known_event' in tables:
         for fkey in inspector.get_foreign_keys(table_name="known_event", referred_table="users"):
-            op.drop_constraint(fkey['name'], 'known_event', type_="foreignkey")
+            if conn.dialect.name == "sqlite":
+                if fkey['name']:
+                    with op.batch_alter_table(table_name='known_event') as bop:
+                        bop.drop_constraint(fkey['name'], type_="foreignkey")
+            else:
+                op.drop_constraint(fkey['name'], 'known_event', type_="foreignkey")

Review comment:
       The main reason was 
   `for fkey in inspector.get_foreign_keys(table_name="known_event", referred_table="users"):` was returning 2 elements, one of which had `fkey['name']` as `None` . And if you call `bop.drop_constraint(fkey['name'], type_="foreignkey")` for sqlite with `fkey['name']` it errors and complains about name not specified while it works for MySQL and Postgres -- so thought would be easier and safe to just keep the current behaviour with Postgres and MySQL.  (didn't investigate further on the foreign key entry with None name)
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org