Set enable_cleanup_closed=True to drop TLS connections without a shutdown

[snemes]

Description

AsyncOpenSearch seems to leak TLS connections due to a missing parameter in aiohttp.TCPConnector.

Issues Resolved

This fixes https://github.com/opensearch-project/opensearch-py/issues/172 and was also fixed "upstream" in this issue https://github.com/elastic/elasticsearch-py/issues/1910. Also seems to be related to this https://github.com/aio-libs/aiohttp/issues/6490 (which is probably the root cause of the issue).

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.

[codecov[bot]]

Codecov Report

Merging #468 (2470d48) into main (dcb79cc) will decrease coverage by 0.07%. Report is 1 commits behind head on main. The diff coverage is 73.33%.

@@            Coverage Diff             @@
##             main     #468      +/-   ##
==========================================
- Coverage   71.69%   71.62%   -0.07%     
==========================================
  Files          87       87              
  Lines        7875     7881       +6     
==========================================
- Hits         5646     5645       -1     
- Misses       2229     2236       +7     

Files	Coverage Δ
opensearchpy/__init__.py	100.00% <ø> (ø)
opensearchpy/_async/helpers/document.py	57.38% <100.00%> (ø)
opensearchpy/_async/helpers/index.py	60.80% <100.00%> (ø)
opensearchpy/_async/http_aiohttp.py	83.33% <100.00%> (ø)
opensearchpy/connection/async_connections.py	95.91% <100.00%> (+0.17%)	:arrow_up:
opensearchpy/connection_pool.py	89.74% <100.00%> (ø)
opensearchpy/helpers/actions.py	44.22% <ø> (ø)
opensearchpy/helpers/asyncsigner.py	95.65% <100.00%> (ø)
opensearchpy/helpers/field.py	93.37% <100.00%> (ø)
opensearchpy/helpers/index.py	64.50% <100.00%> (ø)
... and 6 more

... and 1 file with indirect coverage changes

[dblock]

Thanks! Could you please add a line to CHANGELOG and write a unit test (ideally showing that the leak is fixed, but at least that the parameter is passed through).

[snemes]

Sure, I can add a line to the changelog, but I won't have time to write a unit test for this, sorry about that.

On Thu, Aug 3, 2023, 17:24 Daniel (dB.) Doubrovkine < @.***> wrote:

Thanks! Could you please add a line to CHANGELOG and write a unit test (ideally showing that the leak is fixed, but at least that the parameter is passed through).

— Reply to this email directly, view it on GitHub https://github.com/opensearch-project/opensearch-py/pull/468#issuecomment-1664189798, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABP4VHX5QX7ETYREDSWGW7LXTO7DLANCNFSM6AAAAAA3C4WYHU . You are receiving this because you authored the thread.Message ID: @.***>

[dblock]

Sure, I can add a line to the changelog, but I won't have time to write a unit test for this, sorry about that. … On Thu, Aug 3, 2023, 17:24 Daniel (dB.) Doubrovkine < @.> wrote: Thanks! Could you please add a line to CHANGELOG and write a unit test (ideally showing that the leak is fixed, but at least that the parameter is passed through). — Reply to this email directly, view it on GitHub <#468 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABP4VHX5QX7ETYREDSWGW7LXTO7DLANCNFSM6AAAAAA3C4WYHU . You are receiving this because you authored the thread.Message ID: @.>

We'll have to wait for one to merge this. Thanks for your help, hopefully someone can finish the PR.

[saimedhi]

@snemes, Could you please try to write a test when you have a moment? Thank you!

[dblock]

I tried to concoct a test for this but it became rather convoluted, so I gave up. Merging this as is. The documentation in https://docs.aiohttp.org/en/stable/client_reference.html is convincing enough that this is a harmless improvement that times out unclosed SSL connections and ES client already merged the same including in https://github.com/elastic/elastic-transport-python/commit/57a2c18415c66b1e2ec74ae4daf75d1f5d6f1bde.