Switching to the default connection class to address slower connection with RequestsHttpConnection #411
Conversation
…ction Signed-off-by: beaioun <[email protected]>
There was a problem hiding this comment.
With the new change, it's failing for a unittest that tests client creation with aws credentials. Seems like it's failing in the underlying packages. We'll need to see why this error only occurs when Urllib3HttpConnection is used and not when RequestsHttpConnection is used. Regardless of which connection class is included, AWSV4SignerAuth is used. Therefore, this has to be something related to the pathway that Urllib3HttpConnection invokes.
s = <opensearchpy.helpers.signer.AWSV4SignerAuth object at 0x10b73c940>
def b(s):
> return s.encode("latin-1")
E AttributeError: 'AWSV4SignerAuth' object has no attribute 'encode'
.venv/lib/python3.8/site-packages/urllib3/packages/six.py:687: AttributeError
…lable and will be passed directly to the header without having the former issue. Signed-off-by: Mingyang Shi <[email protected]>
Hey @IanHoang , I figured out where the issue was. The Urllib3HttpConnection has to pair with Urllib3AWSV4SignerAuth for http_auth because that is callable, whereas RequestsAWSV4SignerAuth is not and will try to call make_headers in the urllib3 and throws an error for the subsequent actions. Could you try to run the workflow again? Thanks. |
|
Also, I don't think |
@rishabh6788 I see, I'll update to 2.4.1 see if that works. |
|
FYI, there is bug in |
| self.aws_log_in_dict["service"]) | ||
| return opensearchpy.OpenSearch(hosts=self.hosts, use_ssl=True, verify_certs=True, http_auth=aws_auth, | ||
| connection_class=opensearchpy.RequestsHttpConnection) | ||
| connection_class=opensearchpy.Urllib3HttpConnection) |
There was a problem hiding this comment.
FYI this is now the default and you could remove this option if you want to. Doesn't hurt to keep it either.
There was a problem hiding this comment.
Thank you @dblock , so if we just use opensearch-py-2.4.1 then we don't need to explicitly specify connection_class parameter?
There was a problem hiding this comment.
Yes, but specifying it explicitly doesn't hurt.
This is not the case, it works with AOSS well. The LangChain integration was failing because it checks for |
…rt in os-py client in this version. Signed-off-by: beaioun <[email protected]>
@rishabh6788 I updated the opensearch-py version to 2.4.1 and fixed some other importing issues with this version. All tests passed on my end. 769ff90 |
Description
This PR implements the default Urllib3HttpConnection to replace RequestsHttpConnection to address the slower connection issue, as recommended by the opensearch-py guide for connection classes. Upon checking the code, I discovered this was the only place RequestsHttpConnection was used. Please do point out if I was wrong here.
Issues Resolved
#408
Testing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.