You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2018/09/28 05:30:12 UTC

[GitHub] codelipenghui commented on issue #2671: Simplify ServiceUrlProvider related APIs

codelipenghui commented on issue #2671: Simplify ServiceUrlProvider related APIs
URL: https://github.com/apache/pulsar/pull/2671#issuecomment-425325363
 
 
   πŸ‘
   
   Matteo Merli <no...@github.com> 于2018εΉ΄9月28ζ—₯周五 δΈ‹εˆ1:09ε†™ι“οΌš
   
   > Motivation
   >
   > I have seen few issues with ee5afa5
   > <https://github.com/apache/pulsar/commit/ee5afa5007e260891140956d244a865d73368321>
   > :
   >
   >    1.
   >
   >    There are 3 new methods in the PulsarClient interface which are meant
   >    to be used together. We should reduce
   >    the public API to the minimum possible. So I propose to just have a
   >    Pulsar.updateServiceUrl() which internally
   >    will perform all the needed operations in order to reconnect the
   >    client to new endpoint.
   >    2.
   >
   >    We shouldn't be exposing ClientConfigurationData in the API since it
   >    belongs to impl package
   >    3.
   >
   >    There is a leak of resources when the LookupService gets replaced with
   >    the new one since the existing
   >    is not closed.
   >
   > Modifications
   >
   > Refactored to simplify API for adding ServiceUrlProvider
   >
   > cc/ @codelipenghui <https://github.com/codelipenghui>
   >
   > It would be good to get this into 2.2 since it's changing the API for this
   > new feature.
   > ------------------------------
   > You can view, comment on, or merge this pull request online at:
   >
   >   https://github.com/apache/pulsar/pull/2671
   > Commit Summary
   >
   >    - Simplify ServiceUrlProvider related APIs
   >
   > File Changes
   >
   >    - *M*
   >    pulsar-broker/src/test/java/org/apache/pulsar/client/api/ServiceUrlProviderTest.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-0> (8)
   >    - *M*
   >    pulsar-client/src/main/java/org/apache/pulsar/client/api/PulsarClient.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-1> (35)
   >    - *M*
   >    pulsar-client/src/main/java/org/apache/pulsar/client/api/ServiceUrlProvider.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-2> (25)
   >    - *M*
   >    pulsar-client/src/main/java/org/apache/pulsar/client/impl/BinaryProtoLookupService.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-3> (8)
   >    - *M*
   >    pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientBuilderImpl.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-4> (2)
   >    - *M*
   >    pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionPool.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-5> (20)
   >    - *M*
   >    pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpClient.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-6> (18)
   >    - *M*
   >    pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpLookupService.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-7> (5)
   >    - *M*
   >    pulsar-client/src/main/java/org/apache/pulsar/client/impl/LookupService.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-8> (6)
   >    - *M*
   >    pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java
   >    <https://github.com/apache/pulsar/pull/2671/files#diff-9> (22)
   >
   > Patch Links:
   >
   >    - https://github.com/apache/pulsar/pull/2671.patch
   >    - https://github.com/apache/pulsar/pull/2671.diff
   >
   > β€”
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/pulsar/pull/2671>, or mute the thread
   > <https://github.com/notifications/unsubscribe-auth/AMAkBRQbcliYeaS0MVCFlFW2b88gEQYXks5ufa8QgaJpZM4W93A0>
   > .
   >
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services