You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "gnought (via GitHub)" <gi...@apache.org> on 2023/11/09 15:03:06 UTC
[PR] chore: support selenium 4.10.0+ [superset]
gnought opened a new pull request, #25933:
URL: https://github.com/apache/superset/pull/25933
### SUMMARY
This is a proper fix for the PR https://github.com/apache/superset/pull/24538.
However we have to think about how the `WEBDRIVER_CONFIGURATION` should be. The selenium 4.10+ only allows `options` and `services` to be passed to the `WebDriver` constructor.
https://github.com/SeleniumHQ/selenium/blob/c14d9678990942b93cb421c5567d0da7fb29c7bd/py/selenium/webdriver/firefox/webdriver.py#L41-L46
https://github.com/apache/superset/blob/a698587e8c867f25f4cace6ec02259cccaa5986c/superset/utils/webdriver.py#L276-L279
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "nigzak (via GitHub)" <gi...@apache.org>.
nigzak commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-1957271937
Hi @rusackas
the pull request here seems to update it to V4.10.x - only have in mind it should be updated to >= V4.14.x (newer as it is in this pull request)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2795012459
@rusackas @gnought Am I correct in assuming that you can't change the `firefox_binary` using the configurations after this change? Or am I missing something?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: support selenium 4.10.0+ [superset]
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on code in PR #25933:
URL: https://github.com/apache/superset/pull/25933#discussion_r1390025362
##########
superset/config.py:
##########
@@ -1360,7 +1360,7 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
WEBDRIVER_AUTH_FUNC = None
# Any config options to be passed as-is to the webdriver
-WEBDRIVER_CONFIGURATION: dict[Any, Any] = {"service_log_path": "/dev/null"}
+WEBDRIVER_CONFIGURATION: dict[Any, Any] = {}
Review Comment:
@gnought would you mind providing context for this change? Note irrespective of what the default values are this would still be deemed a breaking change given that the customizable Superset configurations are deemed a public interface.
CC @michael-s-molina for context regarding our recent discussion on breaking changes.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2588233276
bumped it to 4.27.1... let's see if it works! 🤞
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2035047393
> FYI I fixed the merge conflicts, can we remove the hold on this one?
@mistercrunch We can't remove the `hold` label if the PR contains a breaking change. That will only be possible during a breaking window.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-1957258040
This has also been updated to auto-close https://github.com/apache/superset/issues/26992 when it does become mergeable.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2583881476
`./scripts/uv-pip-compile.sh -P {package-you-want-to-update}` is the new way to go. Highly recommend using `uv` locally too in place of pyenv/pip/virtualenv/...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2035037304
FYI I fixed the merge conflicts, can we remove the hold on this one?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "nigzak (via GitHub)" <gi...@apache.org>.
nigzak commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-1956334766
FYI: if the change is made it should be updated to 4.14.x (or newer) to fix a CVE finding (refer linked https://github.com/apache/superset/issues/26992)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2536698076
@gnought we want to push this through for Superset 5.0 (we're approaching the breaking window during which this can be merged). Would you be able to rebase this PR and fix failing CI checks?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2538438468
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/25933?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `21.73913%` with `18 lines` in your changes missing coverage. Please review.
> Project coverage is 65.33%. Comparing base [(`76d897e`)](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`7cc517d`)](https://app.codecov.io/gh/apache/superset/commit/7cc517dac87076b33f2c04c3538f1a7a363b3b17?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1170 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/25933?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/utils/webdriver.py](https://app.codecov.io/gh/apache/superset/pull/25933?src=pr&el=tree&filepath=superset%2Futils%2Fwebdriver.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdXRpbHMvd2ViZHJpdmVyLnB5) | 18.18% | [18 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/25933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #25933 +/- ##
==========================================
+ Coverage 60.48% 65.33% +4.84%
==========================================
Files 1931 537 -1394
Lines 76236 39077 -37159
Branches 8568 0 -8568
==========================================
- Hits 46114 25531 -20583
+ Misses 28017 13546 -14471
+ Partials 2105 0 -2105
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/25933/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/25933/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.73% <21.73%> (-0.44%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/25933/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/25933/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.25% <21.73%> (-0.56%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/25933/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.33% <21.73%> (+1.84%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/25933/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `60.85% <21.73%> (+3.23%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/25933?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "gnought (via GitHub)" <gi...@apache.org>.
gnought commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2538532278
> @gnought we want to push this through for Superset 5.0 (we're approaching the breaking window during which this can be merged). Would you be able to rebase this PR and fix failing CI checks?
@villebro done.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "fisjac (via GitHub)" <gi...@apache.org>.
fisjac commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2598649344
I'm running into issues with generating screenshots with the webdriver following the version bump. Trying to fix locally
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #25933:
URL: https://github.com/apache/superset/pull/25933
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2576475940
@gnought It looks like after a rebase, you'll just need to do `./scripts/uv-pip-compile.sh -P selenium`. You may have to install a bunch of stuff first, e.g.
```bash
$ python3 -m venv venv
$ source venv/bin/activate
$ python3 -m pip install -r requirements/development.txt
$ ./scripts/uv-pip-compile.sh
```
I also had to `pip install uv` personally... not sure why that didn't come along for the ride.
CC @mistercrunch who can fact-check me on any of the above.
Alternatively, if you can give me write access on this fork/branch, I can try to push a commit.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: support selenium 4.10.0+ [superset]
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-1804359147
Thanks @gnought for the change. As your point out with Selenium 4.10+ the `WebDriver` signature changed which means that existing `WEBDRIVER_CONFIGURATION` definitions (including the [default](https://github.com/apache/superset/blob/83b7fa92af3994d2ac0cb296c04e5253f50a96d6/superset/config.py#L1363)) could fail and thus this would be deemed a breaking change. It seems prudent to hold off this change until when the breaking window opens for Superset 4.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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-1957252386
Unfortunately, I think this missed the proposal window for breaking changes, and will have to go into 5.0. I'l add it to that project board now. I'm also not sure where we stand with the potential migration to Playwright (thus tagging @kgabryje here) and if that will become the standard by 5.0. If we want to proceed with this upgrade, which seems sensible, this PR will just need a rebase and bump to 4.14.x or greater by the time that breaking window re-opens.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "gnought (via GitHub)" <gi...@apache.org>.
gnought commented on code in PR #25933:
URL: https://github.com/apache/superset/pull/25933#discussion_r1390446319
##########
superset/config.py:
##########
@@ -1360,7 +1360,7 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
WEBDRIVER_AUTH_FUNC = None
# Any config options to be passed as-is to the webdriver
-WEBDRIVER_CONFIGURATION: dict[Any, Any] = {"service_log_path": "/dev/null"}
+WEBDRIVER_CONFIGURATION: dict[Any, Any] = {}
Review Comment:
@john-bodley
In 4.9.0, `service_log_path` is one of parameters in `WebDriver` constructor.
https://github.com/SeleniumHQ/selenium/blob/bc7e0c7341fdde78cdd80ce572002a7866fd1769/py/selenium/webdriver/firefox/webdriver.py#L90-L110
However it is no longer in 4.10.0+.
https://github.com/SeleniumHQ/selenium/blob/c14d9678990942b93cb421c5567d0da7fb29c7bd/py/selenium/webdriver/firefox/webdriver.py#L50-L53
The following commit will support 4.10.0. It also opens for users to refine Webdrver `Options` and `Service`.
For sure, this will be a breaking change if users defining `WEBDRIVER_CONFIGURATION` themselves using 4.9.0 WebDriver defintion.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on code in PR #25933:
URL: https://github.com/apache/superset/pull/25933#discussion_r1391446594
##########
superset/utils/webdriver.py:
##########
@@ -250,32 +250,54 @@ def get_screenshot(self, url: str, element_name: str, user: User) -> bytes | Non
class WebDriverSelenium(WebDriverProxy):
def create(self) -> WebDriver:
+ # Add additional configured webdriver options
Review Comment:
@gnought thanks for refactoring, however I sense this change isn't suffice. The TL;DR is either:
1. We hold of this change until 4.0 when the breaking window opens, or
2. We need to mutate existing user defined `WEBDRIVER_CONFIGURATION ` to conform to the new options/service structure.
For example in the current form if an implementation has defined the following,
```python
WEBDRIVER_CONFIGURATION = {"service_log_path": "/dev/null"}
```
then said configuration would be lost if this PR was merged because the `service_log_path` key isn't handled. It would be good to look at the Selenium changelog and check the codebase to see whether they have a mechanism for migrating from the old to new `WebDriver` class.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.10.0+ [superset]
Posted by "gnought (via GitHub)" <gi...@apache.org>.
gnought commented on code in PR #25933:
URL: https://github.com/apache/superset/pull/25933#discussion_r1393507960
##########
superset/utils/webdriver.py:
##########
@@ -250,32 +250,54 @@ def get_screenshot(self, url: str, element_name: str, user: User) -> bytes | Non
class WebDriverSelenium(WebDriverProxy):
def create(self) -> WebDriver:
+ # Add additional configured webdriver options
Review Comment:
@john-bodley, modified code a bit for backward compatibility. Hope it makes us feel safe.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "gnought (via GitHub)" <gi...@apache.org>.
gnought commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2583360563
@rusackas I sent you an invite to my fork.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(deps): bump selenium 4.14.0+ [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #25933:
URL: https://github.com/apache/superset/pull/25933#issuecomment-2575978059
We are now good to merge this, but it looks like there's a conflict that needs resolution. I would hand-edit the `base.txt` file to resolve it, but now we use `uv` and `pyproject.toml` for this, so it's a bit different than it was when this was opened.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org