Skip to content
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

Add ClientFactory.numConnections() #3613

Merged
merged 3 commits into from
Jun 14, 2021
Merged

Add ClientFactory.numConnections() #3613

merged 3 commits into from
Jun 14, 2021

Conversation

kojilin
Copy link
Contributor

@kojilin kojilin commented Jun 5, 2021

Motivation:

It's be nice if ClientFactory has numConnections() like Server,
so that a user can easily figure out how many connections it is managing.

Modifications:

  • Add Clientfactory.numConnections(), get connection count from allChannels directly.
  • Add test for multiple h1c request & multie servers

Result:

Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

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

Thank you so much for working on this, @kojilin 🙇

@trustin trustin added this to the 1.9.0 milestone Jun 5, 2021
@codecov
Copy link

codecov bot commented Jun 5, 2021

Codecov Report

Merging #3613 (d01e669) into master (1b6c3b7) will increase coverage by 0.04%.
The diff coverage is 75.00%.

❗ Current head d01e669 differs from pull request most recent head 0fa2bec. Consider uploading reports for the commit 0fa2bec to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3613      +/-   ##
============================================
+ Coverage     73.81%   73.85%   +0.04%     
- Complexity    14356    14381      +25     
============================================
  Files          1260     1263       +3     
  Lines         54820    54878      +58     
  Branches       7022     7021       -1     
============================================
+ Hits          40463    40528      +65     
+ Misses        10785    10779       -6     
+ Partials       3572     3571       -1     
Impacted Files Coverage Δ
...ava/com/linecorp/armeria/client/ClientFactory.java 65.33% <ø> (ø)
...necorp/armeria/client/DecoratingClientFactory.java 37.03% <0.00%> (-1.43%) ⬇️
.../linecorp/armeria/client/DefaultClientFactory.java 72.34% <100.00%> (+2.44%) ⬆️
...a/com/linecorp/armeria/client/HttpChannelPool.java 78.74% <100.00%> (+0.33%) ⬆️
...com/linecorp/armeria/client/HttpClientFactory.java 85.71% <100.00%> (+0.11%) ⬆️
...p/armeria/common/stream/AbstractStreamMessage.java 79.80% <0.00%> (-1.93%) ⬇️
...uth/oauth2/OAuth2TokenIntrospectionAuthorizer.java 63.15% <0.00%> (-1.76%) ⬇️
...inecorp/armeria/server/HttpResponseSubscriber.java 77.89% <0.00%> (-1.06%) ⬇️
...ia/internal/common/stream/ByteBufDecoderInput.java 84.89% <0.00%> (-0.72%) ⬇️
.../linecorp/armeria/client/Http1ResponseDecoder.java 66.66% <0.00%> (-0.61%) ⬇️
... and 26 more

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 1b6c3b7...0fa2bec. Read the comment docs.

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks a lot! @kojilin

Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

👍 👍 👍

@trustin trustin merged commit 35473d2 into line:master Jun 14, 2021
@kojilin kojilin deleted the 3596-2 branch June 15, 2021 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ClientFactory.numConnections()
4 participants