-
Notifications
You must be signed in to change notification settings - Fork 183
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
Implement AsyncOpenSearch() parameter ssl_assert_hostname
#843
Implement AsyncOpenSearch() parameter ssl_assert_hostname
#843
Conversation
…disabling SSL hostname verification Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #843 +/- ##
==========================================
- Coverage 71.95% 70.92% -1.04%
==========================================
Files 91 113 +22
Lines 8001 8900 +899
==========================================
+ Hits 5757 6312 +555
- Misses 2244 2588 +344 ☔ View full report in Codecov by Sentry. |
Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com>
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.
I know it's a fairly trivial change, but we want to make sure someone doesn't break this code in the future, so let's add a test, please?
Mention it somewhere in the user guide in this repo, too.
Fix DCO, rebase please? |
Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com>
4552db2
to
7752471
Compare
Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com>
Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
CI failed in lint, |
Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com>
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.
Looks like a legit test failure with a newer Python version?
_ TestAIOHttpConnection.test_nowarn_when_test_uses_https_if_verify_certs_is_off _
self = <test_opensearchpy.test_async.test_connection.TestAIOHttpConnection object at 0x7f8f401a8740>
async def test_nowarn_when_test_uses_https_if_verify_certs_is_off(self) -> None:
with warnings.catch_warnings(record=True) as w:
con = AIOHttpConnection(
use_ssl=True, verify_certs=False, ssl_show_warn=False
)
await con._create_aiohttp_session()
> assert w == []
E assert [<warnings.WarningMessage object at 0x7f8f304f7ef0>] == []
E
E Left contains one more item: <warnings.WarningMessage object at 0x7f8f304f7ef0>
E
E Full diff:
E - []
E + [
E + <warnings.WarningMessage object at 0x7f8f304f7ef0>,
E + ]
test_opensearchpy/test_async/test_connection.py:231: AssertionError
Testing with Python 3.12 works for me, can you rerun the test? |
Failed again, it's not a fluke, there's some warning but the contents don't show up in the log (maybe start there to see the error) https://github.com/opensearch-project/opensearch-py/actions/runs/11846183040/job/33061554546?pr=843. |
|
OK, so it's a deprecated parameter on |
Yes please, we do want tests to pass :) |
Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com>
Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com>
Thanks @dblock! I don't see where the release schedule for clients is documented, so wondering when this change will be available. |
Open an issue to release v. next and I can try to make it happen this coming week. |
Description
Implements the
ssl_assert_hostname
parameter forAsyncOpenSearch()
.Issues Resolved
Closes #842
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.