Skip to content
This repository has been archived by the owner on Nov 7, 2021. It is now read-only.

Allow inject connector from ElasticSearch till Connection #138

Merged
merged 3 commits into from
Apr 19, 2017

Conversation

pfreixes
Copy link

Related with this PR1[1], it enables inject from the upper layer the adhoc Connection class

[1] #137

@codecov-io
Copy link

codecov-io commented Apr 19, 2017

Codecov Report

Merging #138 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #138      +/-   ##
==========================================
+ Coverage   81.09%   81.15%   +0.06%     
==========================================
  Files          23       23              
  Lines        3533     3545      +12     
  Branches      725      727       +2     
==========================================
+ Hits         2865     2877      +12     
  Misses        389      389              
  Partials      279      279
Impacted Files Coverage Δ
tests/test_transport.py 100% <100%> (ø) ⬆️
aioes/transport.py 83.33% <100%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9e2e6a...54105cb. Read the comment docs.

@pfreixes
Copy link
Author

@popravich can you take a look to this MR?

Also, I would like to ask you for a favor. could you publish a new release with the last two changes?

We are having a lot of issues with the default aiohttp Connector, that uses the default DNS resolver and a forerver cache table. We have implemented our TCPConnector - I expect that this finally will be also merged in the 2.X series of aiohttp, but due to the current architecture of aioes we can not inject our own implementation.

In any case, thanks in advance!

self._loop = loop
self._connector = connector
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here in Transport it should be connector_factory,
otherwise a single connector will be shared between different Connections (ClientSessions)
and when some connection will get dropped underlying ClientSession will close the shared connector
making other connections broken as well.
I think it could look like this:

connector_factory=lambda: None

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, going to change it and covering it with test.

@popravich
Copy link
Contributor

Please add test also.

@pfreixes
Copy link
Author

@popravich done.

@popravich popravich merged commit 573ab52 into aio-libs-abandoned:master Apr 19, 2017
@popravich
Copy link
Contributor

Thanks

@pfreixes
Copy link
Author

you welcome, any chance to get a 0.7.1 release? otherwise, any ETA?

@popravich
Copy link
Contributor

Ok, I will do a release in a several hours

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants