pref: add proxies kwargs in duckduckgo tool run func - #501
Conversation
WalkthroughThe recent update introduces a new Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Apply Sweep Rules to your PR?
This is an automated message generated by Sweep AI. |
There was a problem hiding this comment.
Hello @cavities, thanks for opening your first pull request 😊! We really appreciate your work. Happy Coding 🎉🎊 !
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- promptulate/tools/duckduckgo/api_wrapper.py (3 hunks)
- promptulate/tools/duckduckgo/tools.py (2 hunks)
Additional comments: 5
promptulate/tools/duckduckgo/api_wrapper.py (3)
- 13-13: The addition of the
proxiesattribute with a default value ofNoneis correctly implemented and follows the conventions of optional attributes in the class.- 40-40: The modification to pass
self.proxiesto theDDGSobject in thequeryfunction is correctly implemented. Ensure that theDDGSclass from theduckduckgo_searchpackage supports theproxiesparameter.- 77-77: The modification to pass
self.proxiesto theDDGSobject in thequery_by_formatted_resultsfunction is correctly implemented. Ensure that theDDGSclass from theduckduckgo_searchpackage supports theproxiesparameter, similar to the previous comment.promptulate/tools/duckduckgo/tools.py (2)
- 37-39: The implementation to check for
proxiesinkwargsand setself.api_wrapper.proxiesaccordingly in the_runmethod ofDuckDuckGoToolis correct. Ensure that this approach does not introduce any side effects, especially in concurrent or subsequent calls with different proxy settings.- 85-87: The implementation to check for
proxiesinkwargsand setself.api_wrapper.proxiesaccordingly in the_runmethod ofDuckDuckGoReferenceToolis correct. Ensure that this approach does not introduce any side effects, especially in concurrent or subsequent calls with different proxy settings, similar to the previous comment.
|
You can run |
Undertone0809
left a comment
There was a problem hiding this comment.
Two correctness issues block this as written: the proxy value is passed into DDGS positionally instead of as the proxies kwarg, and the tool mutates self.api_wrapper.proxies without clearing it, so a proxy can leak into later calls on the same tool instance.
| num_results = self.max_num_of_results | ||
|
|
||
| with DDGS() as ddgs: | ||
| with DDGS(self.proxies) as ddgs: |
There was a problem hiding this comment.
duckduckgo_search==3.9.11 defines DDGS(headers=None, proxies=None, timeout=10), so DDGS(self.proxies) binds the proxy string to headers, not proxies. This means the proxy is never applied. Please pass it explicitly as DDGS(proxies=self.proxies) here and in the other call site.
| """ | ||
| from duckduckgo_search.exceptions import RateLimitException | ||
|
|
||
| if "proxies" in kwargs: |
There was a problem hiding this comment.
This mutates self.api_wrapper.proxies only when kwargs["proxies"] is truthy, so once one call sets a proxy, later calls on the same tool instance will keep using that stale proxy unless the process is restarted. It also makes proxies=None unable to clear the setting. Please thread proxies through per call, or at least assign self.api_wrapper.proxies = kwargs.get("proxies") before the query.
.. add proxies kwargs in duckduckgo tool
tool.run(xxx,proxies="socks5://localhost:7891")
..
Summary by CodeRabbit