Re: [PR] chore: remove unnecessary disables [superset]

Fri, 11 Jul 2025 08:28:53 -0700 


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


##########
superset/extensions/pylint.py:
##########
@@ -85,18 +85,18 @@ class SQLParsingLibraryImportChecker(BaseChecker):
     }
 
     def _is_disallowed(self, file_path: Path, root_mod: str) -> bool:
-        # True if sqlglot is imported outside superset/sql,
-        # or if any forbidden library is imported anywhere
+        # Never allow sqlparse/sqloxide
+        if root_mod in {"sqlparse", "sqloxide"}:
+            return True
+
+        # Allow sqlglot inside superset/sql and in the config

Review Comment:
   ### Improve _is_disallowed method rationale documentation <sub>![category 
Documentation](https://img.shields.io/badge/Documentation-7c3aed)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The docstring comment for the _is_disallowed method should explain why 
certain SQL parsing libraries are disallowed in specific locations.
   
   
   ###### Why this matters
   Without understanding why these restrictions exist, developers might 
inadvertently bypass them or remove them, potentially introducing security or 
maintainability issues.
   
   ###### Suggested change ∙ *Feature Preview*
       def _is_disallowed(self, file_path: Path, root_mod: str) -> bool:
           # Restrict sqlparse/sqloxide due to known parsing limitations and 
security concerns.
           # Restrict sqlglot to superset/sql/* to maintain consistent SQL 
parsing across the codebase,
           # with an exception for config.py where parsing rules are defined.
           if root_mod in {"sqlparse", "sqloxide"}:
               return True
   
   
   ###### 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/79898d5e-7e76-414c-bce8-7750e60c5585/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/79898d5e-7e76-414c-bce8-7750e60c5585?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/79898d5e-7e76-414c-bce8-7750e60c5585?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/79898d5e-7e76-414c-bce8-7750e60c5585?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/79898d5e-7e76-414c-bce8-7750e60c5585)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:fe14f7bc-1a31-4a48-b0cf-2e53b6053cca -->
   
   
   [](fe14f7bc-1a31-4a48-b0cf-2e53b6053cca)



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