Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[py] DeprecationWarning raised in default webdriver init #14690

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Oct 30, 2024

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

    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    @VietND96 VietND96 marked this pull request as ready for review October 30, 2024 22:55
    Copy link
    Contributor

    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.

    Copy link
    Contributor

    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 merged commit 961db5e into trunk Oct 31, 2024
    13 checks passed
    @VietND96 VietND96 deleted the python-remote-conn branch October 31, 2024 12:58
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    2 participants