feat(http-manager)!: default to configurable pooling connection manager#384
Merged
dargmuesli merged 7 commits intobetafrom Jan 22, 2024
Merged
feat(http-manager)!: default to configurable pooling connection manager#384dargmuesli merged 7 commits intobetafrom
dargmuesli merged 7 commits intobetafrom
Conversation
in SpotifyApi; if not called, default is BasicHttpClientConnectionManager
and eliminated setConnectionManager()
…lean and restored previous SpotifyApi.build() with no parameter
…oolingConnectionManager
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #384 +/- ##
============================================
+ Coverage 66.08% 66.09% +0.01%
- Complexity 661 664 +3
============================================
Files 183 184 +1
Lines 7103 7100 -3
Branches 1148 1145 -3
============================================
- Hits 4694 4693 -1
- Misses 1506 1507 +1
+ Partials 903 900 -3 ☔ View full report in Codecov by Sentry. |
Member
Author
|
Let's not get too worried about the patch's code coverage 😄 |
Member
|
Got nothing to add either, looks good! I will give the RC a full spin soonish. If I don't report back, assume everything is working fine :D |
|
Tested |
Member
Author
|
Thank you for the feedback! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds one commit to @gahisee's PR #375, which aligns the builder pattern so that it matches way it's used throughout this project: allowing the connection manager to be configurable. It also lets the connection manager default to a pooling connection manager so that most users don't face issue #383.
This is a breaking change (which is not an issue for me, those occur rarely) as the
connectTimeoutConnectionConfigcannot be set anymore as is. Instead, a (basic) connection manager having thatConnectionConfigwould need to be set now if it's needed to be kept.@gahisee @Selbi182 @linde9821 please tell me what you think, I'd release a release candidate for this.
@gahisee your pull request would be merged automatically as well when this one is merged as this PR builds on top of yours so all your stats will be kept. I just thought it's a good idea to have this PR open to see the proposed changes more clearly when comparing against
master💡