Closed benblack86 closed 6 years ago
Merging #639 into develop will decrease coverage by
<.01%
. The diff coverage is84.15%
.
@@ Coverage Diff @@
## develop #639 +/- ##
===========================================
- Coverage 84.75% 84.74% -0.01%
===========================================
Files 99 100 +1
Lines 4440 4518 +78
Branches 407 403 -4
===========================================
+ Hits 3763 3829 +66
- Misses 677 689 +12
Impacted Files | Coverage Δ | |
---|---|---|
...ssus/src/main/scala/colossus/core/Connection.scala | 68.18% <ø> (ø) |
:arrow_up: |
...lossus/src/main/scala/colossus/service/Async.scala | 50% <ø> (ø) |
:arrow_up: |
...sus/src/main/scala/colossus/core/RetryPolicy.scala | 90.9% <ø> (ø) |
:arrow_up: |
...in/scala/colossus/service/AsyncServiceClient.scala | 87.17% <0%> (-7.27%) |
:arrow_down: |
...c/main/scala/colossus/protocols/http/package.scala | 30.76% <100%> (ø) |
:arrow_up: |
...ain/scala/colossus/protocols/http/HttpClient.scala | 88.88% <100%> (-1.12%) |
:arrow_down: |
...src/main/scala/colossus/metrics/ConfigHelper.scala | 92.59% <100%> (+0.92%) |
:arrow_up: |
...ain/scala/colossus/service/ServiceClientPool.scala | 100% <100%> (ø) |
:arrow_up: |
...s/src/main/scala/colossus/service/ServiceDSL.scala | 81.13% <55.55%> (-2.55%) |
:arrow_down: |
...rc/main/scala/colossus/service/ServiceClient.scala | 92.51% <75%> (+0.79%) |
:arrow_up: |
... and 10 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 307f4c4...5ad9fcf. Read the comment docs.
LGTM 👍
Service client with load balancer and retries built in! Includes tests and doc changes.
Overall I feel like this is moving in the correct direction, but I think it can be cleaned up in future PRs. My problem is that we are squishing different functionality into the client trait. I think what we currently call clients, should be renamed connections. A client would then contain one-to-many connections, which it can manage. You can conceive that if you want to skip the client and just talk to the connection, that should be possible. Anyways, that can wait for another day.
@aliyakamercan @jlbelmonte @amotamed @DanSimon