feat(http): expose connection manager #409
Conversation
* add HttpClientConnectionManager to SpotifyHttpManager.builder * add getConnectionManager to handle default
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #409 +/- ##
============================================
- Coverage 66.33% 66.29% -0.05%
Complexity 669 669
============================================
Files 184 184
Lines 7111 7117 +6
Branches 1146 1147 +1
============================================
+ Hits 4717 4718 +1
- Misses 1491 1495 +4
- Partials 903 904 +1 ☔ View full report in Codecov by Sentry. |
| } | ||
| } | ||
|
|
||
| public HttpClientConnectionManager getConnectionManager() { |
There was a problem hiding this comment.
getConnectionManager is no longer necessary
There was a problem hiding this comment.
We have those getters for all class variables right now, like getProxy, ... which we don't use in our code base either, so I just added this as well.
| if (builder.connectionManager == null) { | ||
| BasicHttpClientConnectionManager basicHttpClientConnectionManager = new BasicHttpClientConnectionManager(); | ||
| basicHttpClientConnectionManager.setConnectionConfig(ConnectionConfig | ||
| .custom() | ||
| .setConnectTimeout(connectTimeout != null ? | ||
| Timeout.ofMilliseconds(connectTimeout) : | ||
| ConnectionConfig.DEFAULT.getConnectTimeout()) | ||
| .build()); | ||
| this.connectionManager = basicHttpClientConnectionManager; | ||
| } else { | ||
| this.connectionManager = builder.connectionManager; | ||
| } |
There was a problem hiding this comment.
nitpick consider conditional consistency throughout the library.
either this:
if (value == null) {
// something
} else {
// something else
}
or this:
if (value != null) {
// something
} else {
// something else
}
bruceadowns
left a comment
bruceadowns
left a comment
There was a problem hiding this comment.
lgtm, I like keeping the builder usage consistent.
dargmuesli
force-pushed
the
expose-connmgr-dargmuesli
branch
from
2836747 to
c514b65
Compare
Builds upon #407, keeping the setter-only builder architecture.