-
Notifications
You must be signed in to change notification settings - Fork 608
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
enabling ProxyAgent
take an instance of Agent
as an option
#1998
Conversation
Signed-off-by: Rishabh Bhandari <[email protected]>
Can you add in a test? |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1998 +/- ##
=======================================
Coverage 90.45% 90.45%
=======================================
Files 71 71
Lines 6138 6138
=======================================
Hits 5552 5552
Misses 586 586
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Rishabh Bhandari <[email protected]>
Added the tests for both scenarios, take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add docs for this change?
How is this different from #2003 |
#2003 majorly targets the broader issue of successive requests to ProxyAgent getting blocked, thus introducing a new This pr solves the inability to use ProxyAgent and any other Agent at the same time (example mentioned here, #1957). |
Do you think we could use the same pattern with a factory? |
Closing this as #2003 solves the issue in almost the same way (and that solution has been approved) |
In the updated constructor, the
opts
parameter is checked to see if it contains anagent
property that is an instance ofAgent
. If it is, then the[kAgent]
is set to the provided instance. Otherwise, a newAgent
instance is created and assigned to the[kAgent]
.fixes: #1962