SBIN2010 commented on code in PR #34762:
URL: https://github.com/apache/superset/pull/34762#discussion_r2311941566


##########
superset/migrations/versions/2025-08-13_19-09_da125679ebb8_fix_table_chart_conditional_formatting_.py:
##########
@@ -52,56 +54,156 @@ def upgrade():
     bind = op.get_bind()
     session = db.Session(bind=bind)
 
-    for slc in session.query(Slice).filter(Slice.viz_type == "table"):
-        try:
-            params = json.loads(slc.params)
-            conditional_formatting = params.get("conditional_formatting", [])
-
-            if conditional_formatting:
-                new_conditional_formatting = []
-
-                for formatter in conditional_formatting:
-                    color_scheme = formatter.get("colorScheme")
-
-                    if color_scheme not in ["Green", "Red"]:
-                        new_formatter = formatter.copy()
-                        new_formatter["toAllRow"] = False
-                        new_formatter["toTextColor"] = False
-                        new_conditional_formatting.append(new_formatter)
-                    else:
-                        new_conditional_formatting.append(formatter)
-
-                params["conditional_formatting"] = new_conditional_formatting
-                slc.params = json.dumps(params)
-
-        except (json.JSONDecodeError, TypeError) as e:
-            logger.error(f"Invalid JSON in slice {slc.id} params: {e}")
-            continue
-
-    session.commit()
-    session.close()
+    try:
+        total_count = session.query(Slice).filter(Slice.viz_type == 
"table").count()
+        logger.info(f"Found {total_count} table slices to process")
+
+        batch_size = 1000
+        processed = 0
+
+        while processed < total_count:
+            try:
+                slices_batch = (
+                    session.query(Slice)
+                    .filter(Slice.viz_type == "table")
+                    .offset(processed)
+                    .limit(batch_size)
+                    .all()
+                )
+
+                if not slices_batch:
+                    break
+
+                for slc in slices_batch:
+                    try:
+                        params = json.loads(slc.params)
+                        conditional_formatting = params.get(
+                            "conditional_formatting", []
+                        )
+
+                        if conditional_formatting:
+                            new_conditional_formatting = []
+                            for formatter in conditional_formatting:
+                                color_scheme = formatter.get("colorScheme")
+
+                                if color_scheme not in ["Green", "Red"]:
+                                    new_conditional_formatting.append(
+                                        {
+                                            **formatter,
+                                            "toAllRow": False,
+                                            "toTextColor": False,
+                                        }
+                                    )
+                                else:
+                                    
new_conditional_formatting.append(formatter)
+
+                            params["conditional_formatting"] = (
+                                new_conditional_formatting
+                            )
+                            slc.params = json.dumps(params)
+
+                    except Exception as e:
+                        logger.error(f"Error processing slice {slc.id}: 
{str(e)}")
+
+                        continue
+
+                session.commit()
+                processed += len(slices_batch)
+                logger.info(f"Processed {processed}/{total_count} slices")
+
+            except SQLAlchemyError as e:
+                session.rollback()
+                logger.error(
+                    f"Error processing batch {processed}-{processed + 
batch_size}: "
+                    f"{str(e)}"
+                )
+                processed += batch_size
+
+        logger.info("Migration completed successfully")
+
+    except Exception as e:
+        session.rollback()
+        logger.error(f"Unexpected error in migration: {str(e)}")
+        raise
+
+    finally:
+        session.close()
 
 
 def downgrade():
     bind = op.get_bind()
-    session = db.Session(bind=bind)
-
-    for slc in session.query(Slice).filter(Slice.viz_type == "table"):
-        params = json.loads(slc.params)
-        conditional_formatting = params.get("conditional_formatting", [])
-        if conditional_formatting:
-            new_conditional_formatting = []
-            for formatter in conditional_formatting:
-                if "toAllRow" in formatter or "toTextColor" in formatter:
-                    new_formatter = formatter.copy()
-                    new_formatter.pop("toAllRow", None)
-                    new_formatter.pop("toTextColor", None)
-                    new_conditional_formatting.append(new_formatter)
-                else:
-                    new_conditional_formatting.append(formatter)
-
-            params["conditional_formatting"] = new_conditional_formatting
-            slc.params = json.dumps(params)
-
-    session.commit()
-    session.close()
+    session = Session(bind=bind)
+
+    try:
+        total_count = session.query(Slice).filter(Slice.viz_type == 
"table").count()
+        logger.info(f"Found {total_count} table slices to process for 
downgrade")
+
+        batch_size = 1000
+        processed = 0
+
+        while processed < total_count:
+            try:
+                slices_batch = (
+                    session.query(Slice)
+                    .filter(Slice.viz_type == "table")
+                    .offset(processed)
+                    .limit(batch_size)
+                    .all()
+                )
+
+                if not slices_batch:
+                    break
+
+                for slc in slices_batch:
+                    try:
+                        params = json.loads(slc.params)
+                        conditional_formatting = params.get(
+                            "conditional_formatting", []
+                        )
+
+                        if conditional_formatting:
+                            new_conditional_formatting = []
+                            for formatter in conditional_formatting:
+                                if (
+                                    "toAllRow" in formatter
+                                    or "toTextColor" in formatter
+                                ):
+                                    new_formatter = formatter.copy()
+                                    new_formatter.pop("toAllRow", None)
+                                    new_formatter.pop("toTextColor", None)

Review Comment:
   Corrected



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