Re: [PR] fix(Charts.exportTable): download the whole chart when Export to full .CSV is clicked [superset]

Tue, 20 May 2025 04:57:13 -0700 


korbit-ai[bot] commented on code in PR #33541:
URL: https://github.com/apache/superset/pull/33541#discussion_r2097764606


##########
docker/pythonpath_dev/superset_config.py:
##########
@@ -98,8 +98,7 @@ class CeleryConfig:
 
 
 CELERY_CONFIG = CeleryConfig
-
-FEATURE_FLAGS = {"ALERT_REPORTS": True}
+FEATURE_FLAGS = {"ALERT_REPORTS": True, "ALLOW_ADHOC_SUBQUERY": True, 
"ALLOW_FULL_CSV_EXPORT": True}

Review Comment:
   ### Hardcoded Feature Flags <sub>![category 
Design](https://img.shields.io/badge/Design-0d9488)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Feature flags are defined as a hardcoded dictionary without a centralized 
configuration management approach.
   
   
   ###### Why this matters
   This makes it difficult to manage features across different environments and 
lacks flexibility for runtime configuration changes.
   
   ###### Suggested change ∙ *Feature Preview*
   Implement a feature flag configuration system:
   ```python
   FEATURE_FLAGS = {
       **DEFAULT_FEATURE_FLAGS,
       **get_environment_feature_flags(),  # Function to load from env vars
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2aefb2ba-4713-4c8b-b3d6-f4e4e53ca1aa/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2aefb2ba-4713-4c8b-b3d6-f4e4e53ca1aa?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2aefb2ba-4713-4c8b-b3d6-f4e4e53ca1aa?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2aefb2ba-4713-4c8b-b3d6-f4e4e53ca1aa?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2aefb2ba-4713-4c8b-b3d6-f4e4e53ca1aa)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:d22a92fe-bac3-4141-960f-5512f1ba361b -->
   
   
   [](d22a92fe-bac3-4141-960f-5512f1ba361b)



##########
superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts:
##########
@@ -191,7 +197,13 @@ const buildQuery: BuildQuery<TableChartFormData> = (
 
     const moreProps: Partial<QueryObject> = {};
     const ownState = options?.ownState ?? {};
-    if (formDataCopy.server_pagination) {
+    // Only apply server pagination if this is not a full export 
+    // (checking for parameters that indicate we're doing a full export)
+    const isFullExport = 
+      formDataCopy.result_format === 'csv' && 
+      (formDataCopy.row_limit === 0 || formDataCopy.is_rowcount === undefined);

Review Comment:
   ### Incorrect Full Export Detection Logic <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The condition for checking full export includes 'is_rowcount === undefined' 
which could incorrectly identify regular queries as full exports.
   
   
   ###### Why this matters
   When regular table queries are made without the export flag, is_rowcount 
will also be undefined, leading to incorrect pagination behavior and 
potentially returning all rows when not intended.
   
   ###### Suggested change ∙ *Feature Preview*
   Modify the condition to only check for CSV format and row_limit of 0:
   ```typescript
   const isFullExport = formDataCopy.result_format === 'csv' && 
formDataCopy.row_limit === 0;
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/21747e8f-1be5-47a4-934b-1a764d18268c/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/21747e8f-1be5-47a4-934b-1a764d18268c?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/21747e8f-1be5-47a4-934b-1a764d18268c?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/21747e8f-1be5-47a4-934b-1a764d18268c?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/21747e8f-1be5-47a4-934b-1a764d18268c)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:0dff2717-1359-47f7-b965-4347627b3f10 -->
   
   
   [](0dff2717-1359-47f7-b965-4347627b3f10)



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