Re: [PR] fix: Consider default catalog when getting tables and view lists [superset]

Wed, 25 Jun 2025 22:11:22 -0700 


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


##########
superset/models/core.py:
##########
@@ -801,6 +801,7 @@ def get_all_table_names_in_schema(
         :param force: whether to force refresh the cache
         :return: The table/schema pairs
         """
+        catalog = catalog or self.get_default_catalog()

Review Comment:
   ### Default Catalog Behavior Not Documented <sub>![category 
Documentation](https://img.shields.io/badge/Documentation-7c3aed)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The default catalog assignment occurs after the docstring, which may lead to 
confusion since the docstring states 'catalog: optional catalog name' without 
mentioning the default behavior.
   
   
   ###### Why this matters
   Future developers reading the docstring may not be aware that a default 
catalog will be assigned when catalog is None, potentially leading to incorrect 
assumptions about the method's behavior.
   
   ###### Suggested change ∙ *Feature Preview*
   Update the docstring to reflect the default catalog behavior:
   ```python
   def get_all_table_names_in_schema(
       self,
       catalog: str | None,
       schema: str,
   ) -> set[tuple[str, str, str | None]]:
       """Parameters need to be passed as keyword arguments.
   
       For unused parameters, they are referenced in
       cache_util.memoized_func decorator.
   
       :param catalog: optional catalog name, if None defaults to database's 
default catalog
       :param schema: schema name
       :param cache: whether cache is enabled for the function
       :param cache_timeout: timeout in seconds for the cache
       :param force: whether to force refresh the cache
       :return: The table/schema pairs
       """
   ```
   
   
   ###### 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/4789f66c-0dfc-4bdb-b936-597b93e15c5e/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4789f66c-0dfc-4bdb-b936-597b93e15c5e?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/4789f66c-0dfc-4bdb-b936-597b93e15c5e?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/4789f66c-0dfc-4bdb-b936-597b93e15c5e?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4789f66c-0dfc-4bdb-b936-597b93e15c5e)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:cca0de50-79d6-4197-b678-6d4bda715903 -->
   
   
   [](cca0de50-79d6-4197-b678-6d4bda715903)



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