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: [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]
