korbit-ai[bot] commented on code in PR #34440:
URL: https://github.com/apache/superset/pull/34440#discussion_r2246641614
##########
superset/utils/webdriver.py:
##########
@@ -245,47 +245,99 @@ def get_screenshot( # pylint: disable=too-many-locals,
too-many-statements # n
class WebDriverSelenium(WebDriverProxy):
+ def _create_firefox_driver(
+ self, pixel_density: float
+ ) -> tuple[type[WebDriver], type[Service], dict[str, Any]]:
+ """Create Firefox driver configuration."""
+ options = firefox.options.Options()
+ profile = FirefoxProfile()
+ profile.set_preference("layout.css.devPixelsPerPx", str(pixel_density))
+ options.profile = profile
+ return (
+ firefox.webdriver.WebDriver,
+ firefox.service.Service,
+ {"options": options},
+ )
+
+ def _create_chrome_driver(
+ self, pixel_density: float
+ ) -> tuple[type[WebDriver], type[Service], dict[str, Any]]:
+ """Create Chrome driver configuration."""
+ options = chrome.options.Options()
+ options.add_argument(f"--force-device-scale-factor={pixel_density}")
+
options.add_argument(f"--window-size={self._window[0]},{self._window[1]}")
+ return (
+ chrome.webdriver.WebDriver,
+ chrome.service.Service,
+ {"options": options},
+ )
+
+ def _normalize_timeout_values(self, config: dict[str, Any]) -> dict[str,
Any]:
+ """Convert timeout values to float for urllib3 2.x compatibility."""
+ timeout_keys = [
+ "timeout",
+ "connect_timeout",
+ "socket_timeout",
+ "read_timeout",
+ "page_load_timeout",
+ "implicit_wait",
+ "command_executor_timeout",
+ "connection_timeout",
+ ]
+
+ for key, value in config.items():
+ if any(timeout_key in key.lower() for timeout_key in timeout_keys):
Review Comment:
### Over-permissive Timeout Key Matching <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The timeout key matching is too permissive. Using 'in' for substring
matching could lead to false positives if a config key contains a timeout word
but isn't actually a timeout setting.
###### Why this matters
Config keys like 'timeout_description' or 'non_timeout_value' would be
incorrectly identified as timeout values and converted to floats, potentially
breaking the application configuration.
###### Suggested change ∙ *Feature Preview*
Use exact key matching or a more specific pattern matching approach:
```python
for key, value in config.items():
if key.lower() in [t.lower() for t in timeout_keys]:
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/efeffc24-c81d-4458-b1c8-bace15b6520c/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/efeffc24-c81d-4458-b1c8-bace15b6520c?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/efeffc24-c81d-4458-b1c8-bace15b6520c?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/efeffc24-c81d-4458-b1c8-bace15b6520c?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/efeffc24-c81d-4458-b1c8-bace15b6520c)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:aa08acd5-6c33-4e2a-80f6-3a57db023c45 -->
[](aa08acd5-6c33-4e2a-80f6-3a57db023c45)
--
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]
