AutoGPT icon indicating copy to clipboard operation
AutoGPT copied to clipboard

Improvement in web commands functionalities

Open amirdaaee opened this issue 10 months ago • 8 comments

Background

this pr is to address feature requests in #7067 and #7066. in summary it adds two functionalities:

  • adding support for alternative duckduckgo_search SDK supported backends (as described here).
  • support for proxy in selenium for read_web command

Changes 🏗️

  • autogpts/autogpt/autogpt/config/config.py: added duckduckgo_backend and selenium_proxy attributes to Config class to control new feature behavior
  • autogpts/autogpt/autogpt/commands/web_search.py: added backend argument in calling for text() method of DDGS in web_search function
  • autogpts/autogpt/autogpt/commands/web_selenium.py: added --proxy-server argument in web-drive options
  • autogpts/autogpt/.env.template: added documents about duckduckgo_backend and selenium_proxy variables

PR Quality Scorecard ✨

  • [x] Have you used the PR description template?   +2 pts
  • [ ] Is your pull request atomic, focusing on a single change?   +5 pts
  • [x] Have you linked the GitHub issue(s) that this PR addresses?   +5 pts
  • [x] Have you documented your changes clearly and comprehensively?   +5 pts
  • [x] Have you changed or added a feature?   -4 pts
    • [x] Have you added/updated corresponding documentation?   +4 pts
    • [x] Have you added/updated corresponding integration tests?   +5 pts
  • [ ] Have you changed the behavior of AutoGPT?   -5 pts
    • [ ] Have you also run agbenchmark to verify that these changes do not regress performance?   +10 pts

amirdaaee avatar Apr 03 '24 09:04 amirdaaee

Deploy Preview for auto-gpt-docs canceled.

Name Link
Latest commit f393613be25117eb96d94d46ede4d5bd6ea5f96e
Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/66a213fd57ab660008593624

netlify[bot] avatar Apr 03 '24 09:04 netlify[bot]

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 53.79%. Comparing base (62c420e) to head (f393613). Report is 65 commits behind head on master.

Files Patch % Lines
forge/forge/components/web/selenium.py 50.00% 1 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7068      +/-   ##
==========================================
- Coverage   53.81%   53.79%   -0.02%     
==========================================
  Files         124      124              
  Lines        7021     7032      +11     
  Branches      909      912       +3     
==========================================
+ Hits         3778     3783       +5     
- Misses       3110     3114       +4     
- Partials      133      135       +2     
Flag Coverage Δ
Linux 53.58% <71.42%> (+0.01%) :arrow_up:
Windows ?
autogpt-agent 34.09% <ø> (+0.10%) :arrow_up:
forge 58.02% <71.42%> (-0.03%) :arrow_down:
macOS 52.90% <71.42%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 03 '24 09:04 codecov[bot]

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

github-actions[bot] avatar Apr 22 '24 17:04 github-actions[bot]

@CodiumAI-Agent /review

Swiftyos avatar Apr 23 '24 14:04 Swiftyos

PR Review

⏱️ Estimated effort to review [1-5]

3, because the PR involves multiple files and introduces new functionalities which require understanding the existing configuration and command structures. The changes are moderate in complexity, involving backend integrations and proxy settings which need careful review to ensure they are implemented correctly and securely.

🧪 Relevant tests

No

🔍 Possible issues

Possible Bug: The duckduckgo_backend variable is fetched from agent.legacy_config but there is no check to ensure that legacy_config or duckduckgo_backend is not None before using it. This could lead to AttributeError if legacy_config is not properly configured.

Possible Bug: The proxy configuration in web_selenium.py directly appends the proxy server argument without validating the format or existence of the proxy variable. This might lead to incorrect browser behavior if the proxy string is malformed.

🔒 Security concerns

No

Code feedback:
relevant fileautogpts/autogpt/autogpt/commands/web_search.py
suggestion      

Add a null check for ddc_backend before using it in the DDGS().text method to prevent potential runtime errors if the configuration is missing. [important]

relevant linequery, max_results=num_results, backend=ddc_backend

relevant fileautogpts/autogpt/autogpt/commands/web_selenium.py
suggestion      

Validate the format of the proxy variable to ensure it includes the necessary protocol (e.g., http:// or https://). This will prevent potential issues with browser proxy configuration. [important]

relevant lineoptions.add_argument("--proxy-server=%s" % proxy)


✨ Review tool usage guide:

Overview: The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

  • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

CodiumAI-Agent avatar Apr 23 '24 14:04 CodiumAI-Agent

Sorry for the long wait, this needs updated to component based. Most of it should move over pretty easily. @kcze can answer any questions you have

ntindle avatar Jun 11 '24 01:06 ntindle

Hey @amirdaaee, thanks for the contribution. I've just made a PR request on your branch if you merge it then the PR should be ready to merge into AutoGPT's master. I've merged master into your branch and moved the code so it works with the new component-based architecture. You can read more about it here: https://docs.agpt.co/forge/components/introduction/

kcze avatar Jul 03 '24 10:07 kcze

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

github-actions[bot] avatar Jul 06 '24 09:07 github-actions[bot]