mistercrunch commented on code in PR #30017:
URL: https://github.com/apache/superset/pull/30017#discussion_r1731968324


##########
superset/migrations/versions/2024-08-13_15-27_e53fd48cc078_remove_sl_dataset_users.py:
##########
@@ -34,9 +34,14 @@
 
 def upgrade():
     connection = op.get_bind()
-    if connection.dialect.name != "sqlite":
-        drop_fks_for_table("sl_dataset_users")
-    op.drop_table("sl_dataset_users")
+
+    try:
+        if connection.dialect.name != "sqlite":
+            drop_fks_for_table("sl_dataset_users")
+        op.drop_table("sl_dataset_users")
+    except sa.exc.NoSuchTableError:

Review Comment:
   Whatever makes sense, maybe `drop_table(table_name, including_fks=[], 
only_if_it_exists=True)` or maybe it's for another PR, but a common issue is a 
migration that fails at step 2 and when you rerun it it fails at step one since 
step one can only run once (say a create table). Personally think we could use 
more of the "drop-table-but-only-if-it-exists" and 
"create-table-but-drop-it-first-if-it-exists" type utils to avoid getting 
people tangled up. We've had many people open issue with the message of "step 1 
one of the migration fails, here's the stacktrace", only to realize that the 
issue was with a subsequent step.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to