Skip to content

[py] DeprecationWarning raised in default webdriver init#14690

Merged
VietND96 merged 2 commits intotrunkfrom
python-remote-conn
Oct 31, 2024
Merged

[py] DeprecationWarning raised in default webdriver init#14690
VietND96 merged 2 commits intotrunkfrom
python-remote-conn

Conversation

@VietND96
Copy link
Copy Markdown
Member

@VietND96 VietND96 commented Oct 30, 2024 •
edited
Loading

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Wherever inherits from RemoteConnection, __init__ should pass remote_server_addr to a ClientConfig instance

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix, Enhancement

Description

  • Refactored the initialization of remote connections for Chromium, Firefox, Safari, and IE WebDrivers to use ClientConfig.
  • Removed redundant parameters in superclass initialization for remote connections.
  • Addressed deprecation warnings by ensuring consistent use of ClientConfig.

Changes walkthrough 📝

Relevant files
Enhancement
remote_connection.py
Refactor Chromium remote connection initialization             

py/selenium/webdriver/chromium/remote_connection.py

  • Added default initialization for client_config using ClientConfig.
  • Removed redundant parameters in superclass initialization.
    		•  +1/-2     
    remote_connection.py
    Refactor Firefox remote connection initialization               

    py/selenium/webdriver/firefox/remote_connection.py

  • Added default initialization for client_config using ClientConfig.
  • Removed redundant parameters in superclass initialization.
    		•  +1/-2     
    webdriver.py
    Update IE WebDriver to use ClientConfig                                   

    py/selenium/webdriver/ie/webdriver.py

  • Imported ClientConfig.
  • Used ClientConfig for RemoteConnection initialization.
    		•  +3/-2     
    remote_connection.py
    Refactor Safari remote connection initialization                 

    py/selenium/webdriver/safari/remote_connection.py

  • Added default initialization for client_config using ClientConfig.
  • Removed redundant parameters in superclass initialization.
    		•  +1/-2     
    webdriver.py
    Update Safari WebDriver to use ClientConfig                           

    py/selenium/webdriver/safari/webdriver.py

  • Imported ClientConfig.
  • Used ClientConfig for SafariRemoteConnection initialization.
    		•  +3/-2     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @VietND96
    [py] DeprecationWarning raised in default webdriver init
    2be33ae 
    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    @VietND96 VietND96 marked this pull request as ready for review October 30, 2024 22:55
    @qodo-code-review
    Copy link
    Copy Markdown
    Contributor

    qodo-code-review Bot commented Oct 30, 2024

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Potential Redundancy
    The client_config parameter is being passed to the superclass constructor, but it's also being used to create a new ClientConfig object. This might lead to unexpected behavior if a non-None client_config is passed to the constructor.

    Unused Import
    The ClientConfig is imported but not used directly in the class. Consider moving the import closer to where it's used or removing it if it's not needed.

    @qodo-code-review
    Copy link
    Copy Markdown
    Contributor

    qodo-code-review Bot commented Oct 30, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Separate configuration object creation from its usage for improved code structure

    Consider moving the ClientConfig initialization outside of the RemoteConnection call
    for better readability and potential reuse.

    py/selenium/webdriver/ie/webdriver.py [54-57]

    +client_config = ClientConfig(remote_server_addr=self.service.service_url, keep_alive=keep_alive)
 executor = RemoteConnection(
     ignore_proxy=options._ignore_local_proxy,
-    client_config=ClientConfig(remote_server_addr=self.service.service_url, keep_alive=keep_alive),
+    client_config=client_config,
 )
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion improves code readability by separating the creation of the ClientConfig object from its usage. It makes the code cleaner and potentially allows for reuse of the client_config object, which is a good practice.

    		7

    💡 Need additional feedback ? start a PR chat

    @VietND96 VietND96 requested a review from diemol October 30, 2024 23:00
    @VietND96 VietND96 mentioned this pull request Oct 31, 2024
    Merged
    8 tasks
    @VietND96 VietND96 added the C-py Python Bindings label Oct 31, 2024
    @VietND96 VietND96 merged commit 961db5e into trunk Oct 31, 2024
    @VietND96 VietND96 deleted the python-remote-conn branch October 31, 2024 12:58
    @dependabot dependabot Bot mentioned this pull request Sep 18, 2025
    Merged
    @dependabot dependabot Bot mentioned this pull request Oct 1, 2025
    Closed
    @dependabot dependabot Bot mentioned this pull request Oct 24, 2025
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @diemol diemol diemol approved these changes

    @AutomatedTester AutomatedTester Awaiting requested review from AutomatedTester

    Assignees

    No one assigned

    Labels

    C-py Python Bindings Review effort [1-5]: 2

    Projects

    None yet

    Milestone

    No milestone

    2 participants

    @VietND96 @diemol