korbit-ai[bot] commented on code in PR #34144:
URL: https://github.com/apache/superset/pull/34144#discussion_r2201967671
##########
superset/views/base.py:
##########
@@ -292,6 +293,51 @@
}
+def get_theme_bootstrap_data() -> dict[str, Any]:
+ """
+ Returns the theme data to be sent to the client.
+ """
+ # Get theme configs
+ default_theme = (
+ conf["THEME_DEFAULT"]()
+ if callable(conf["THEME_DEFAULT"])
+ else conf["THEME_DEFAULT"]
+ )
+ dark_theme = (
+ conf["THEME_DARK"]() if callable(conf["THEME_DARK"]) else
conf["THEME_DARK"]
+ )
+ theme_settings = (
+ conf["THEME_SETTINGS"]()
+ if callable(conf["THEME_SETTINGS"])
+ else conf["THEME_SETTINGS"]
+ )
+
+ # Validate and warn if invalid
+ if not is_valid_theme(default_theme):
+ logger.warning("Invalid THEME_DEFAULT configuration, using empty
theme")
+ default_theme = {}
+
+ if not is_valid_theme(dark_theme):
+ logger.warning("Invalid THEME_DARK configuration, using empty theme")
+ dark_theme = {}
Review Comment:
### Missing Valid Theme Fallback Structure <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
When an invalid dark theme is detected, the code sets it to an empty
dictionary rather than a valid default theme structure, which could cause
client-side rendering issues.
###### Why this matters
Using an empty dictionary as a fallback for an invalid theme configuration
may lead to incomplete or broken theme rendering on the client side, as the
theme structure expected by the frontend would be missing.
###### Suggested change ∙ *Feature Preview*
Instead of using an empty dictionary, provide a minimal valid theme
structure as fallback:
```python
if not is_valid_theme(dark_theme):
logger.warning("Invalid THEME_DARK configuration, using default theme
structure")
dark_theme = {
"colors": {},
"typography": {},
"borderRadius": 0,
"spacing": {}
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bc8bfb2-cf25-4ea3-a2b6-664bf0a344fe/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bc8bfb2-cf25-4ea3-a2b6-664bf0a344fe?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bc8bfb2-cf25-4ea3-a2b6-664bf0a344fe?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bc8bfb2-cf25-4ea3-a2b6-664bf0a344fe?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1bc8bfb2-cf25-4ea3-a2b6-664bf0a344fe)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:d827cd69-f1d9-4154-ab1c-f595e61bfbc1 -->
[](d827cd69-f1d9-4154-ab1c-f595e61bfbc1)
##########
superset/views/base.py:
##########
@@ -292,6 +293,51 @@
}
+def get_theme_bootstrap_data() -> dict[str, Any]:
+ """
+ Returns the theme data to be sent to the client.
+ """
+ # Get theme configs
+ default_theme = (
+ conf["THEME_DEFAULT"]()
+ if callable(conf["THEME_DEFAULT"])
+ else conf["THEME_DEFAULT"]
+ )
+ dark_theme = (
+ conf["THEME_DARK"]() if callable(conf["THEME_DARK"]) else
conf["THEME_DARK"]
+ )
+ theme_settings = (
+ conf["THEME_SETTINGS"]()
+ if callable(conf["THEME_SETTINGS"])
+ else conf["THEME_SETTINGS"]
+ )
Review Comment:
### Repetitive config value resolution pattern <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Repeated pattern of checking if config value is callable and executing it if
so. This pattern appears 3 times with similar structure.
###### Why this matters
The repetitive pattern makes the code harder to read and maintain. A utility
function would make the intent clearer and reduce cognitive load.
###### Suggested change ∙ *Feature Preview*
Extract to a helper function:
```python
def get_config_value(key: str) -> Any:
value = conf[key]
return value() if callable(value) else value
# Then use as:
default_theme = get_config_value("THEME_DEFAULT")
dark_theme = get_config_value("THEME_DARK")
theme_settings = get_config_value("THEME_SETTINGS")
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/268a3164-a9bb-4266-8abc-64f83ad8a2aa/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/268a3164-a9bb-4266-8abc-64f83ad8a2aa?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/268a3164-a9bb-4266-8abc-64f83ad8a2aa?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/268a3164-a9bb-4266-8abc-64f83ad8a2aa?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/268a3164-a9bb-4266-8abc-64f83ad8a2aa)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:0f32d74c-8af8-4190-b830-1426c0de8d42 -->
[](0f32d74c-8af8-4190-b830-1426c0de8d42)
##########
superset/views/base.py:
##########
@@ -292,6 +293,51 @@
}
+def get_theme_bootstrap_data() -> dict[str, Any]:
+ """
+ Returns the theme data to be sent to the client.
+ """
+ # Get theme configs
+ default_theme = (
+ conf["THEME_DEFAULT"]()
+ if callable(conf["THEME_DEFAULT"])
+ else conf["THEME_DEFAULT"]
+ )
+ dark_theme = (
+ conf["THEME_DARK"]() if callable(conf["THEME_DARK"]) else
conf["THEME_DARK"]
+ )
+ theme_settings = (
+ conf["THEME_SETTINGS"]()
+ if callable(conf["THEME_SETTINGS"])
+ else conf["THEME_SETTINGS"]
+ )
+
+ # Validate and warn if invalid
+ if not is_valid_theme(default_theme):
+ logger.warning("Invalid THEME_DEFAULT configuration, using empty
theme")
+ default_theme = {}
+
+ if not is_valid_theme(dark_theme):
+ logger.warning("Invalid THEME_DARK configuration, using empty theme")
+ dark_theme = {}
+
+ if not is_valid_theme_settings(theme_settings):
+ logger.warning("Invalid THEME_SETTINGS configuration, using defaults")
+ theme_settings = {
+ "enforced": False,
+ "allowSwitching": True,
+ "allowOSPreference": True,
+ }
Review Comment:
### Hardcoded theme settings defaults <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Magic default values are embedded directly in the code without being defined
as constants or configuration values.
###### Why this matters
Hardcoded default values make it difficult to understand their significance
and make future changes more error-prone.
###### Suggested change ∙ *Feature Preview*
Define defaults as constants:
```python
DEFAULT_THEME_SETTINGS = {
"enforced": False,
"allowSwitching": True,
"allowOSPreference": True,
}
# Then use as:
theme_settings = DEFAULT_THEME_SETTINGS
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/328e078c-6b73-4b20-806f-d84fc2411f32/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/328e078c-6b73-4b20-806f-d84fc2411f32?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/328e078c-6b73-4b20-806f-d84fc2411f32?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/328e078c-6b73-4b20-806f-d84fc2411f32?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/328e078c-6b73-4b20-806f-d84fc2411f32)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:8f899d76-7c0f-4f53-9c82-51a1dc5287f8 -->
[](8f899d76-7c0f-4f53-9c82-51a1dc5287f8)
##########
superset/themes/utils.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Any, Dict
+
+from superset.themes.types import ThemeMode, ThemeSettingsKey
+
+
+def _is_valid_theme_mode(mode: str) -> bool:
+ """Check if a string is a valid theme mode"""
+ try:
+ ThemeMode(mode)
+ return True
+ except ValueError:
+ return False
+
+
+def _is_valid_algorithm(algorithm: Any) -> bool:
Review Comment:
### Overly permissive type hint <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The function uses 'Any' type hint for the algorithm parameter, which is too
permissive and obscures the expected input types.
###### Why this matters
Using 'Any' makes it harder for developers to understand what types are
actually valid without reading the implementation.
###### Suggested change ∙ *Feature Preview*
Use a more specific type hint:
```python
from typing import Union, List
def _is_valid_algorithm(algorithm: Union[str, List[str]]) -> bool:
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/917acad8-52e3-49e0-9ccf-6574c044f41a/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/917acad8-52e3-49e0-9ccf-6574c044f41a?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/917acad8-52e3-49e0-9ccf-6574c044f41a?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/917acad8-52e3-49e0-9ccf-6574c044f41a?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/917acad8-52e3-49e0-9ccf-6574c044f41a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:ffc4585b-4bdd-410c-aa19-32e48601fc59 -->
[](ffc4585b-4bdd-410c-aa19-32e48601fc59)
##########
superset/themes/utils.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Any, Dict
+
+from superset.themes.types import ThemeMode, ThemeSettingsKey
+
+
+def _is_valid_theme_mode(mode: str) -> bool:
+ """Check if a string is a valid theme mode"""
+ try:
+ ThemeMode(mode)
+ return True
+ except ValueError:
+ return False
+
+
+def _is_valid_algorithm(algorithm: Any) -> bool:
+ """Helper function to validate theme algorithm"""
+ if isinstance(algorithm, str):
+ return _is_valid_theme_mode(algorithm) or algorithm == ThemeMode.SYSTEM
+ elif isinstance(algorithm, list):
+ return all(
+ isinstance(alg, str) and _is_valid_theme_mode(alg) for alg in
algorithm
+ )
+ else:
+ return False
+
+
+def is_valid_theme(theme: Dict[str, Any]) -> bool:
+ """Check if a theme is valid"""
+ try:
+ if not isinstance(theme, dict):
+ return False
+
+ # Empty dict is valid
+ if not theme:
+ return True
+
+ # Validate each field type
+ validations = [
+ ("token", dict),
+ ("components", dict),
+ ("hashed", bool),
+ ("inherit", bool),
+ ]
+
+ for field, expected_type in validations:
+ if field in theme and not isinstance(theme[field], expected_type):
+ return False
+
+ # Validate algorithm field separately due to its complexity
+ if "algorithm" in theme and not
_is_valid_algorithm(theme["algorithm"]):
+ return False
+
+ return True
+ except Exception:
+ return False
+
+
+def is_valid_theme_settings(settings: Dict[str, Any]) -> bool:
+ """Check if theme settings are valid"""
+ try:
+ if not isinstance(settings, dict):
+ return False
+
+ # Empty dict is valid
+ if not settings:
+ return True
+
+ # Check if all keys are valid
+ valid_keys = {setting.value for setting in ThemeSettingsKey}
Review Comment:
### Inefficient repeated set creation from static enum <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Creating a new set of valid keys on every function call is inefficient as
ThemeSettingsKey is an enumeration that doesn't change at runtime.
###### Why this matters
Repeatedly creating this set for each validation adds unnecessary
computation overhead, especially if the function is called frequently.
###### Suggested change ∙ *Feature Preview*
Move the valid keys set creation to module level as a constant:
```python
VALID_THEME_SETTINGS = {setting.value for setting in ThemeSettingsKey}
def is_valid_theme_settings(settings: Dict[str, Any]) -> bool:
# Use VALID_THEME_SETTINGS instead of creating new set
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be2627bd-7c04-4781-8fb9-d74e201612ba/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be2627bd-7c04-4781-8fb9-d74e201612ba?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be2627bd-7c04-4781-8fb9-d74e201612ba?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be2627bd-7c04-4781-8fb9-d74e201612ba?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be2627bd-7c04-4781-8fb9-d74e201612ba)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:f414d907-4ac7-48ae-8d72-25c7d788f9ad -->
[](f414d907-4ac7-48ae-8d72-25c7d788f9ad)
##########
superset-frontend/src/types/bootstrapTypes.ts:
##########
@@ -143,6 +147,18 @@
};
}
+export interface SerializableThemeSettings {
+ enforced?: boolean;
+ allowSwitching?: boolean;
+ allowOSPreference?: boolean;
+}
+
+export interface BootstrapThemeDataConfig {
+ default: SerializableThemeConfig | {};
+ dark: SerializableThemeConfig | {};
+ settings: SerializableThemeSettings | {};
+}
Review Comment:
### Invalid Theme Configuration Type Definition <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The union type with empty object ({}) in theme configurations allows for
potentially invalid theme data to be passed through the type system.
###### Why this matters
Empty objects could be passed instead of valid theme configurations, leading
to runtime errors when the theme system attempts to use the configuration.
###### Suggested change ∙ *Feature Preview*
Remove the empty object option to ensure only valid theme configurations are
accepted:
```typescript
export interface BootstrapThemeDataConfig {
default: SerializableThemeConfig;
dark: SerializableThemeConfig;
settings: SerializableThemeSettings;
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/107d67b6-5b1c-4323-8e13-8176ba58a43b/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/107d67b6-5b1c-4323-8e13-8176ba58a43b?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/107d67b6-5b1c-4323-8e13-8176ba58a43b?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/107d67b6-5b1c-4323-8e13-8176ba58a43b?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/107d67b6-5b1c-4323-8e13-8176ba58a43b)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:ed4154c9-cd44-4a1f-ae99-59feed20cb82 -->
[](ed4154c9-cd44-4a1f-ae99-59feed20cb82)
##########
superset/themes/utils.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Any, Dict
+
+from superset.themes.types import ThemeMode, ThemeSettingsKey
+
+
+def _is_valid_theme_mode(mode: str) -> bool:
+ """Check if a string is a valid theme mode"""
+ try:
+ ThemeMode(mode)
+ return True
+ except ValueError:
+ return False
+
+
+def _is_valid_algorithm(algorithm: Any) -> bool:
+ """Helper function to validate theme algorithm"""
+ if isinstance(algorithm, str):
+ return _is_valid_theme_mode(algorithm) or algorithm == ThemeMode.SYSTEM
+ elif isinstance(algorithm, list):
+ return all(
+ isinstance(alg, str) and _is_valid_theme_mode(alg) for alg in
algorithm
+ )
+ else:
+ return False
+
+
+def is_valid_theme(theme: Dict[str, Any]) -> bool:
+ """Check if a theme is valid"""
Review Comment:
### Incomplete theme validation docstring <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The docstring lacks information about the expected theme structure and
validation rules.
###### Why this matters
Developers need to read through the implementation to understand what makes
a theme valid, making the code harder to use correctly.
###### Suggested change ∙ *Feature Preview*
"""Validate theme dictionary structure and types.
A valid theme can be empty or must contain properly typed fields:
- token (dict)
- components (dict)
- hashed (bool)
- inherit (bool)
- algorithm (str or list of str matching ThemeMode)
Returns:
bool: True if theme structure is valid, False otherwise
"""
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/dd750467-f48d-40ef-8624-26c2a3a35277/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/dd750467-f48d-40ef-8624-26c2a3a35277?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/dd750467-f48d-40ef-8624-26c2a3a35277?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/dd750467-f48d-40ef-8624-26c2a3a35277?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/dd750467-f48d-40ef-8624-26c2a3a35277)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:3ac493a8-a79d-4348-ac9b-68d262c66836 -->
[](3ac493a8-a79d-4348-ac9b-68d262c66836)
##########
superset-frontend/src/types/bootstrapTypes.ts:
##########
@@ -143,6 +147,18 @@ export interface MenuData {
};
}
+export interface SerializableThemeSettings {
+ enforced?: boolean;
+ allowSwitching?: boolean;
+ allowOSPreference?: boolean;
+}
Review Comment:
### Missing theme settings interface documentation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The interface SerializableThemeSettings lacks documentation explaining the
purpose and impact of its configuration flags.
###### Why this matters
Without clear documentation, developers may incorrectly configure theme
settings, leading to unexpected theming behavior in the application.
###### Suggested change ∙ *Feature Preview*
/**
* Theme configuration settings that control theme behavior.
* - enforced: Forces the use of the default theme
* - allowSwitching: Enables manual theme switching
* - allowOSPreference: Enables automatic theme switching based on OS
preferences
*/
export interface SerializableThemeSettings {
enforced?: boolean;
allowSwitching?: boolean;
allowOSPreference?: boolean;
}
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1a6f35c2-d14d-4092-85b8-87403370e7c7/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1a6f35c2-d14d-4092-85b8-87403370e7c7?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1a6f35c2-d14d-4092-85b8-87403370e7c7?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1a6f35c2-d14d-4092-85b8-87403370e7c7?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1a6f35c2-d14d-4092-85b8-87403370e7c7)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:1dc05624-4302-4256-99a0-4678f911ce9e -->
[](1dc05624-4302-4256-99a0-4678f911ce9e)
##########
superset/views/base.py:
##########
@@ -292,6 +293,51 @@
}
+def get_theme_bootstrap_data() -> dict[str, Any]:
+ """
+ Returns the theme data to be sent to the client.
+ """
+ # Get theme configs
+ default_theme = (
+ conf["THEME_DEFAULT"]()
+ if callable(conf["THEME_DEFAULT"])
+ else conf["THEME_DEFAULT"]
+ )
+ dark_theme = (
+ conf["THEME_DARK"]() if callable(conf["THEME_DARK"]) else
conf["THEME_DARK"]
+ )
+ theme_settings = (
+ conf["THEME_SETTINGS"]()
+ if callable(conf["THEME_SETTINGS"])
+ else conf["THEME_SETTINGS"]
+ )
+
+ # Validate and warn if invalid
+ if not is_valid_theme(default_theme):
+ logger.warning("Invalid THEME_DEFAULT configuration, using empty
theme")
+ default_theme = {}
+
+ if not is_valid_theme(dark_theme):
+ logger.warning("Invalid THEME_DARK configuration, using empty theme")
+ dark_theme = {}
+
+ if not is_valid_theme_settings(theme_settings):
+ logger.warning("Invalid THEME_SETTINGS configuration, using defaults")
Review Comment:
### Insufficient Error Context in Theme Validation Logs <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Warning logs for invalid theme configurations lack context about what made
the theme invalid, making troubleshooting difficult.
###### Why this matters
Without detailed error information in the logs, administrators would
struggle to identify and fix incorrect theme configurations.
###### Suggested change ∙ *Feature Preview*
```python
if not is_valid_theme(default_theme):
logger.warning(
"Invalid THEME_DEFAULT configuration: %s, using empty theme",
default_theme
)
default_theme = {}
if not is_valid_theme(dark_theme):
logger.warning(
"Invalid THEME_DARK configuration: %s, using empty theme",
dark_theme
)
dark_theme = {}
if not is_valid_theme_settings(theme_settings):
logger.warning(
"Invalid THEME_SETTINGS configuration: %s, using defaults",
theme_settings
)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68982dc7-8810-4817-a5d5-03042e60f062/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68982dc7-8810-4817-a5d5-03042e60f062?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68982dc7-8810-4817-a5d5-03042e60f062?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68982dc7-8810-4817-a5d5-03042e60f062?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68982dc7-8810-4817-a5d5-03042e60f062)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:d89e5f12-4069-4143-93e3-bffd287a0853 -->
[](d89e5f12-4069-4143-93e3-bffd287a0853)
##########
superset/themes/utils.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Any, Dict
+
+from superset.themes.types import ThemeMode, ThemeSettingsKey
+
+
+def _is_valid_theme_mode(mode: str) -> bool:
+ """Check if a string is a valid theme mode"""
Review Comment:
### Insufficient docstring for theme mode validation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The docstring only describes what the function does, but not the expected
input/output or why it's needed.
###### Why this matters
Without understanding valid theme modes and their purpose, developers may
misuse the function or have to dig through other files to understand its usage.
###### Suggested change ∙ *Feature Preview*
"""Validate if a string represents a valid theme mode.
Args:
mode: String to validate against ThemeMode enum
Returns:
bool: True if mode is a valid ThemeMode value, False otherwise
"""
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9975ca46-eb9d-4b4c-b5d6-cc45e21d784a/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9975ca46-eb9d-4b4c-b5d6-cc45e21d784a?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9975ca46-eb9d-4b4c-b5d6-cc45e21d784a?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9975ca46-eb9d-4b4c-b5d6-cc45e21d784a?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9975ca46-eb9d-4b4c-b5d6-cc45e21d784a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:025b67b9-d52b-480f-abb9-e713e4da5e4f -->
[](025b67b9-d52b-480f-abb9-e713e4da5e4f)
--
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]
