mistercrunch commented on code in PR #34762:
URL: https://github.com/apache/superset/pull/34762#discussion_r2311680326
##########
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:
Review Comment:
when getting to >8 indents in it usually time means it's to write a method
##########
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:
Review Comment:
when getting to >8 indents in it usually time means it's to write a method
--
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]
