abderbejaoui commented on code in PR #38490:
URL: https://github.com/apache/superset/pull/38490#discussion_r3531475492


##########
superset/databases/schemas.py:
##########
@@ -277,6 +277,48 @@ def extra_validator(value: str) -> str:
                         )
                     ]
                 )
+
+        # Only validate when the key is present. An absent key means "unset"
+        # (the cache is simply not configured), which is valid. When the key
+        # is present it must be a mapping — this rejects an explicit ``null``
+        # as well as other non-dict values (0, "", []), keeping the API in
+        # sync with ``ImportV1DatabaseExtraSchema`` (whose ``fields.Dict`` also
+        # rejects null) and avoiding a stored ``null`` that would break the
+        # ``Database.metadata_cache_timeout`` accessors.
+        if "metadata_cache_timeout" in extra_:
+            metadata_cache_timeout = extra_["metadata_cache_timeout"]
+            if not isinstance(metadata_cache_timeout, dict):
+                raise ValidationError(
+                    [
+                        _(
+                            "The metadata_cache_timeout must be a mapping from 
"
+                            "string keys to non-negative integer values."
+                        )
+                    ]
+                )
+            for key in (
+                "schema_cache_timeout",
+                "table_cache_timeout",
+                "catalog_cache_timeout",
+            ):
+                # An absent key is unset (valid). When the key is present the
+                # value must be a non-negative integer — a present ``null`` is
+                # rejected here too, matching the import schema (its
+                # ``fields.Integer`` has no ``allow_none``) and preventing an
+                # enabled-but-None state in the model accessors.
+                if key not in metadata_cache_timeout:
+                    continue
+                timeout = metadata_cache_timeout[key]
+                if not isinstance(timeout, int) or timeout < 0:

Review Comment:
   Good catch — fixed in `1221c121e7`. Since Python's `bool` is a subclass of 
`int`, `true`/`false` were slipping through the integer check as `1`/`0`. The 
validator now rejects booleans explicitly:
   
   ```python
   if (
       isinstance(timeout, bool)
       or not isinstance(timeout, int)
       or timeout < 0
   ):
   



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