AutoGPT icon indicating copy to clipboard operation
AutoGPT copied to clipboard

Set Selenium Chrome binary and driver path in .env config

Open primaryobjects opened this issue 2 years ago • 8 comments
trafficstars

Background

  • The path to chrome.exe and the Selenium chromedriver may exist in different locations (Linux, Mac, Windows). This change allows setting the path in the .env instead of hard-coding the path.

Changes

  • Added config to set Selenium Chrome binary and driver path in .env

Documentation

  • Documented within .env.template.
## CHROMIUM_BINARY_LOCATION - Path to chrome.exe for use with selenium (Optional).
## CHROMIUM_DRIVER_PATH - Path to chrome driver. See https://chromedriver.chromium.org/downloads (default: /usr/bin/chromedriver)

Test Plan

  • Unit tested and ran locally successfully.

PR Quality Checklist

  • [X] My pull request is atomic and focuses on a single change.
  • [X] I have thoroughly tested my changes with multiple different prompts.
  • [X] I have considered potential risks and mitigations for my changes.
  • [X] I have documented my changes clearly and comprehensively.
  • [X] I have not snuck in any "extra" small tweaks changes

primaryobjects avatar May 03 '23 19:05 primaryobjects

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 21, 2023 10:38am

vercel[bot] avatar May 03 '23 19:05 vercel[bot]

Codecov Report

Patch coverage: 100.00% and project coverage change: -2.56 :warning:

Comparison is base (bcc32cc) 63.82% compared to head (b4d1cff) 61.27%.

:exclamation: Current head b4d1cff differs from pull request most recent head 29c2476. Consider uploading reports for the commit 29c2476 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3756      +/-   ##
==========================================
- Coverage   63.82%   61.27%   -2.56%     
==========================================
  Files          74       73       -1     
  Lines        3447     3331     -116     
  Branches      507      480      -27     
==========================================
- Hits         2200     2041     -159     
- Misses       1079     1152      +73     
+ Partials      168      138      -30     
Impacted Files Coverage Δ
autogpt/commands/web_selenium.py 55.43% <100.00%> (-29.01%) :arrow_down:
autogpt/config/config.py 75.14% <100.00%> (+1.28%) :arrow_up:

... and 22 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar May 03 '23 19:05 codecov[bot]

This is a mass message from the AutoGPT core team. Our apologies for the ongoing delay in processing PRs. This is because we are re-architecting the AutoGPT core!

For more details (and for infor on joining our Discord), please refer to: https://github.com/Significant-Gravitas/Auto-GPT/wiki/Architecting

p-i- avatar May 05 '23 00:05 p-i-

Note this looks like a useful change to be reviewed/integrated despite the ongoing re-arch effort, it's primarily config-level, with very few code changes (and those primarily are about removing hard-coded stuff using best practices like a config dict instead), also it seems to come with testing. So definitely do consider for review/integration to help close some PRs. Thanks

Boostrix avatar May 07 '23 07:05 Boostrix

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

github-actions[bot] avatar May 13 '23 22:05 github-actions[bot]

Deployment failed with the following error:

Resource is limited - try again in 1 hour (more than 100, code: "api-deployments-free-per-day").

vercel[bot] avatar May 14 '23 01:05 vercel[bot]

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

github-actions[bot] avatar May 14 '23 01:05 github-actions[bot]

Deployment failed with the following error:

Resource is limited - try again in 55 minutes (more than 100, code: "api-deployments-free-per-day").

vercel[bot] avatar May 14 '23 01:05 vercel[bot]

I want to test this pr

jimmylegendary avatar May 19 '23 17:05 jimmylegendary

As I tested this PR, there is no problem except test_selenium.py as I commented

But, for next task, we might need to not use executable_path because this parameter is deprecated after selenium4. We can use Service object in place of using executable_path.

jimmylegendary avatar May 20 '23 09:05 jimmylegendary

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

github-actions[bot] avatar May 26 '23 17:05 github-actions[bot]

Thanks for submitting! It was a challenge getting browsing to work reliably across multiple platforms, and I wish we had picked up on this PR earlier. By now, it has been addressed in a slightly different way afaics: https://github.com/Significant-Gravitas/Auto-GPT/blob/7c4fc45b4a536e6423b6491c34953a2516863d78/autogpt/commands/web_selenium.py#L134-L139

Please close the PR if you agree that this solves the problem, or let me know otherwise! :)

Pwuts avatar Jul 14 '23 00:07 Pwuts

I'm ok with closing this PR and deferring it to a later time, if needed.

Note, the proposed alternative still appears to hardcode the path to chromedriver. It also does not allow configuring a custom path to the chrome.exe binary.

This PR addresses both issues by allowing the user to customize the path locations - notably on Windows, where the path to the driver and executable may differ.

It's ok either way, we have the PR and can always reopen if needed. Thank you.

primaryobjects avatar Jul 14 '23 01:07 primaryobjects

Note, the proposed alternative still appears to hardcode the path to chromedriver.

Yup, because that's the location of the driver in the release docker image and on Linux. On Windows, it will not be in that location, and the program will manage by downloading a driver on the fly. This is a much simpler solution than making everything configurable, hence the proposal to close this PR. Unless of course the current implementation causes problems somehow.

It also does not allow configuring a custom path to the chrome.exe binary.

What would be the use case for such a config option?

Pwuts avatar Jul 15 '23 01:07 Pwuts