Skip to content

Conversation

@cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Jun 4, 2025

User description

🔗 Related Issues

#15849

💥 What does this PR do?

This PR removes the ability to set SE_SAFARIDRIVER environment variable to control the location of safaridriver, since it is not possible to change its location. This brings the Python bindings into alignment with others (i.e. Java bindings) that do not respect this environment variable.

Note: you can still pass a key name in the constructor to specify driver location, but I didn't want to change the public API. However, setting a key name is not useful because you can't have safaridriver in an alternate location.


PR Type

Enhancement


Description

  • Remove support for SE_SAFARIDRIVER environment variable

  • Align Python Safari driver behavior with other bindings

  • Simplify Service class initialization logic


Changes walkthrough 📝

Relevant files
Enhancement
service.py
Remove SE_SAFARIDRIVER env var support from Safari Service

py/selenium/webdriver/safari/service.py

  • Removed default use of SE_SAFARIDRIVER env var for driver path
  • Cleaned up constructor logic for driver path environment key
  • +0/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added the C-py Python Bindings label Jun 4, 2025
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 4, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    @cgoldberg
    Copy link
    Member Author

    Closing this... The proper fix is to remove support for setting safaridriver location via environment variable completely since the driver can only exist in its default location. That will be a change to the API.

    @cgoldberg cgoldberg closed this Jun 4, 2025
    @cgoldberg cgoldberg deleted the py-remove-safari-var branch June 4, 2025 13:40
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants