aminghadersohi commented on code in PR #36933:
URL: https://github.com/apache/superset/pull/36933#discussion_r2932056295


##########
superset/config.py:
##########
@@ -730,7 +731,7 @@ class D3TimeFormat(TypedDict, total=False):
     # Enable sharing charts with embedding
     # @lifecycle: stable
     # @category: runtime_config
-    "EMBEDDABLE_CHARTS": True,
+    "EMBEDDABLE_CHARTS": False,

Review Comment:
   Thanks for flagging this. The `EMBEDDABLE_CHARTS` feature flag is set to 
`True` by default in the current code (`superset/config.py:739`), so this 
concern has been addressed.



##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -39,6 +40,117 @@
 from superset.mcp_service.utils.url_utils import get_superset_base_url
 from superset.utils import json
 
+# =============================================================================
+# Visualization Type Constants
+# =============================================================================
+
+# Time series viz types that require a datetime column on the x-axis
+TIMESERIES_VIZ_TYPES = {
+    "echarts_timeseries",
+    "echarts_timeseries_line",
+    "echarts_timeseries_bar",
+    "echarts_timeseries_area",
+    "echarts_timeseries_scatter",
+    "echarts_timeseries_smooth",
+    "echarts_timeseries_step",
+    "mixed_timeseries",

Review Comment:
   Good catch. The code already uses `echarts_area` (not 
`echarts_timeseries_area`) in all viz type sets and mappings. This was 
corrected in a prior revision.



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