-
-
Notifications
You must be signed in to change notification settings - Fork 289
added setConnectionManager(bPooling) to SpotifyApi.Builder #375
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
added setConnectionManager(bPooling) to SpotifyApi.Builder #375
Conversation
in SpotifyApi; if not called, default is BasicHttpClientConnectionManager
dargmuesli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your suggestion! Some code changes would need to be done though 🙌
| * A HttpManager Builder ready to build SpotifyHttpManager | ||
| */ | ||
| public static final IHttpManager DEFAULT_HTTP_MANAGER = new SpotifyHttpManager.Builder().build(); | ||
| public static final SpotifyHttpManager.Builder shmb = new SpotifyHttpManager.Builder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please try to name variables without abbreviations.
and eliminated setConnectionManager()
…lean and restored previous SpotifyApi.build() with no parameter
|
Please respond to the threads when making changes, indicating which threads can be resolved so I can verify and resolve 🙌 |
|
I am getting this error too much, looking good. When do you plan to release? |
|
@gahisee what do you think about just always using the pooling connection manager? We could maybe save on some complexity by just using that instead of the basic connection manager, or do you see any breaking changes? |
|
@ulpcan have you ever tried out the changes proposed in this PR in a project of yours or similar to confirm them to be working as is or have you just approved the changes without checking? |
|
@dargmuesli i always like having options. With one call, the user can opt for pooling manager; |
|
See #384/files for my proposal, based on and including this PR, on how to implement the fix using the builder pattern. |

if bPooling is true, PoolingHttpClientConnectionManager is used.
if not called, default is BasicHttpClientConnectionManager.
This addition allows for certain applications where multiple concurrent requests are made.
Otherwise, the following exception will occur:
java.lang.IllegalStateException: Connection 192.168.1.13:35776<->35.186.224.25:443 is still allocated
at org.apache.hc.core5.util.Asserts.check(Asserts.java:50)