You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mistercrunch (via GitHub)" <gi...@apache.org> on 2025/07/24 07:52:35 UTC

[PR] fix: address numerous long-standing console errors (python & web) [superset]

mistercrunch opened a new pull request, #34299:
URL: https://github.com/apache/superset/pull/34299

   ## Summary
   - fixing console errors related to aggrid, improve i18n for it
   - tackled various annoying console issues in python
   - tackled some browser console warning


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix: address numerous long-standing console errors (python & web) [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch merged PR #34299:
URL: https://github.com/apache/superset/pull/34299


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix: address numerous long-standing console errors (python & web) [superset]

Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #34299:
URL: https://github.com/apache/superset/pull/34299#discussion_r2227743837


##########
superset/db_engine_specs/__init__.py:
##########
@@ -154,7 +154,7 @@ def get_available_engine_specs() -> dict[type[BaseEngineSpec], set[str]]:  # noq
         try:
             dialect = ep.load()
         except Exception as ex:  # pylint: disable=broad-except
-            logger.warning("Unable to load SQLAlchemy dialect %s: %s", ep.name, ex)
+            logger.debug("Unable to load SQLAlchemy dialect %s: %s", ep.name, ex)

Review Comment:
   ### Inappropriate Log Level for Critical Error <sub>![category Error Handling](https://img.shields.io/badge/Error%20Handling-ea580c)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Changing log level from warning to debug for dialect loading failures reduces visibility of potentially important errors.
   
   
   ###### Why this matters
   Dialect loading failures could indicate broken dependencies or configuration issues that need attention. Using debug level may cause these issues to go unnoticed in production.
   
   ###### Suggested change ∙ *Feature Preview*
   Maintain the warning level for better visibility of dialect loading issues:
   ```python
   except Exception as ex:  # pylint: disable=broad-except
       logger.warning("Unable to load SQLAlchemy dialect %s: %s", ep.name, ex)
   ```
   
   
   ###### 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/503b5fe9-1228-4672-b09d-91247377ca23/upvote) [![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/503b5fe9-1228-4672-b09d-91247377ca23?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/503b5fe9-1228-4672-b09d-91247377ca23?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/503b5fe9-1228-4672-b09d-91247377ca23?what_not_in_standard=true) [![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/503b5fe9-1228-4672-b09d-91247377ca23)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:218801df-f118-4fc6-aa34-614fe11863d4 -->
   
   
   [](218801df-f118-4fc6-aa34-614fe11863d4)



##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -326,6 +326,79 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps> = memo(
           paginationPageSizeSelector={PAGE_SIZE_OPTIONS}
           suppressDragLeaveHidesColumns
           pinnedBottomRowData={showTotals ? [cleanedTotals] : undefined}
+          localeText={{
+            // Pagination controls
+            next: t('Next'),
+            previous: t('Previous'),
+            page: t('Page'),
+            more: t('More'),
+            to: t('to'),
+            of: t('of'),
+            first: t('First'),
+            last: t('Last'),
+            loadingOoo: t('Loading...'),
+            // Set Filter
+            selectAll: t('Select All'),
+            searchOoo: t('Search...'),
+            blanks: t('Blanks'),
+            // Filter operations
+            filterOoo: t('Filter'),
+            applyFilter: t('Apply Filter'),
+            equals: t('Equals'),
+            notEqual: t('Not Equal'),
+            lessThan: t('Less Than'),
+            greaterThan: t('Greater Than'),
+            lessThanOrEqual: t('Less Than or Equal'),
+            greaterThanOrEqual: t('Greater Than or Equal'),
+            inRange: t('In Range'),
+            contains: t('Contains'),
+            notContains: t('Not Contains'),
+            startsWith: t('Starts With'),
+            endsWith: t('Ends With'),
+            // Logical conditions
+            andCondition: t('AND'),
+            orCondition: t('OR'),
+            // Panel and group labels
+            group: t('Group'),
+            columns: t('Columns'),
+            filters: t('Filters'),
+            valueColumns: t('Value Columns'),
+            pivotMode: t('Pivot Mode'),
+            groups: t('Groups'),
+            values: t('Values'),
+            pivots: t('Pivots'),
+            toolPanelButton: t('Tool Panel'),
+            // Enterprise menu items
+            pinColumn: t('Pin Column'),
+            valueAggregation: t('Value Aggregation'),
+            autosizeThiscolumn: t('Autosize This Column'),
+            autosizeAllColumns: t('Autosize All Columns'),
+            groupBy: t('Group By'),
+            ungroupBy: t('Ungroup By'),
+            resetColumns: t('Reset Columns'),
+            expandAll: t('Expand All'),
+            collapseAll: t('Collapse All'),
+            toolPanel: t('Tool Panel'),
+            export: t('Export'),
+            csvExport: t('CSV Export'),
+            excelExport: t('Excel Export'),
+            excelXmlExport: t('Excel XML Export'),
+            // Aggregation functions
+            sum: t('Sum'),
+            min: t('Min'),
+            max: t('Max'),
+            none: t('None'),
+            count: t('Count'),
+            average: t('Average'),
+            // Standard menu items
+            copy: t('Copy'),
+            copyWithHeaders: t('Copy with Headers'),
+            paste: t('Paste'),
+            // Column menu and sorting
+            sortAscending: t('Sort Ascending'),
+            sortDescending: t('Sort Descending'),
+            sortUnSort: t('Clear Sort'),
+          }}

Review Comment:
   ### Large inline translation object obscures component logic <sub>![category Readability](https://img.shields.io/badge/Readability-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The inline localeText object with 60+ translations creates visual noise and makes the component harder to read.
   
   
   ###### Why this matters
   Large inline configuration objects make the component's structure difficult to scan and understand, increasing cognitive load for developers.
   
   ###### Suggested change ∙ *Feature Preview*
   Extract localeText translations to a separate constant:
   ```typescript
   // localeConstants.ts
   export const AG_GRID_LOCALE_TEXT = {
     next: t('Next'),
     previous: t('Previous'),
     // ... other translations
   };
   
   // In component:
   localeText={AG_GRID_LOCALE_TEXT}
   ```
   
   
   ###### 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/1ce2992e-c1f3-4892-8598-80496f9ff6f3/upvote) [![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1ce2992e-c1f3-4892-8598-80496f9ff6f3?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/1ce2992e-c1f3-4892-8598-80496f9ff6f3?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/1ce2992e-c1f3-4892-8598-80496f9ff6f3?what_not_in_standard=true) [![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1ce2992e-c1f3-4892-8598-80496f9ff6f3)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:3c092949-9366-49d8-893e-1df861f80423 -->
   
   
   [](3c092949-9366-49d8-893e-1df861f80423)



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org