[GitHub] [superset] john-bodley commented on a diff in pull request #21262: fix(sqllab): table list exceeds MAX_TABLE_NAMES

[GitBox <gi...@apache.org>]

john-bodley commented on code in PR #21262:
URL: https://github.com/apache/superset/pull/21262#discussion_r958966251


##########
superset/views/core.py:
##########
@@ -1202,11 +1202,8 @@ def is_match(src: str, target: utils.DatasourceName) -> bool:
 
         max_items = config["MAX_TABLE_NAMES"] or len(tables)
         total_items = len(tables) + len(views)
-        max_tables = len(tables)
-        max_views = len(views)
-        if total_items and substr_parsed:
-            max_tables = max_items * len(tables) // total_items
-            max_views = max_items * len(views) // total_items
+        max_tables = max_items or len(tables)

Review Comment:
   @justinpark this logic isn't quite right, the `//` ensures that `max_tables` + `max_views` = `max_items`. 
   
   Furthermore this endpoint is only invoked [here](https://github.com/apache/superset/blob/master/superset-frontend/src/components/TableSelector/index.tsx#L211-L250) which implies that `substr_parsed` is never truthy and the `MAX_TABLE_NAMES` config doesn't actually get invoked. 
   
   I think we should actually refactor this method to remove the `substr_parsed` logic and deprecate the `MAX_TABLE_NAMES`, especially given that it's non deterministic in terms of how it truncates the list.
   
   Furthermore it seems like `schema_parsed` is always truthy (assuming I'm reading the frontend code correctly), i.e., per line #212 `currentSchema` is truthy and never "undefined" which simplifies the logic and thus large swaths of this method can be refactored. 



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