madhushreeag commented on code in PR #45868:
URL: https://github.com/apache/superset/pull/45868#discussion_r4419883648


##########
superset/models/core.py:
##########
@@ -462,29 +462,36 @@ def parameters_schema(self) -> dict[str, Any]:
     def metadata_cache_timeout(self) -> dict[str, Any]:
         return self.get_extra().get("metadata_cache_timeout", {})
 
+    def _metadata_cache_timeout_value(self, key: str) -> int | None:

Review Comment:
   `int()` is looser than both the API validator (non-negative int, no bools) 
and #45790's migration (ASCII digit strings only). For example, `" 60"`, 
`"+60"`, `"1_000"`, and `True` are all accepted. `"-1"` becomes 
`CACHE_DISABLED_TIMEOUT` and silently turns caching off, and 
`int(float("inf"))` raises an uncaught `OverflowError`. Should we match 
existing rules? 
   
   maybe something like 
   ```
   raw = self.metadata_cache_timeout.get(key)
   if isinstance(raw, bool):
        return None
   if isinstance(raw, int):
        return raw if raw >= 0 else None
   if isinstance(raw, str) and raw.isascii() and raw.isdigit():
        return int(raw)
   return None
   ```



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