BewareMyPower commented on PR #170:
URL: 
https://github.com/apache/pulsar-client-cpp/pull/170#issuecomment-1396500856

   > The initialize and close methods are redundant.
   
   I reviewed the `ServiceUrlProvider` in Java client again. **I have thought 
`getServiceUrl()` is called each time the service URL is needed (e.g. topic 
lookup) but it's actually not.** The update of the service URL is actually done 
by `PulsarClient#updateServiceUrl`, which is triggered by the periodical task 
of `ServiceUrlProvide#initialize`.
   
   If we want to implement `AutoClusterFailover` and 
`ControlledClusterFailover` in future, we actually don't need to implement a 
function that returns the service URL. Instead, we only need to implement the 
following methods:
   - PulsarClient#updateServiceUrl
   - PulsarClientImpl#updateAuthentication
   - PulsarClientImpl#updateTlsTrustCertsFilePath
   
   The way of using `ServiceUrlProvider` is bad. It's more like a provider that 
can modify the internal state of `PulsarClient`.
   
   For C++ client, this PR implements the similar way with Java that the 
`ServiceUrlProvider` (a function that returns the service URL) is only 
triggerred when constructing the `Client` instance. There will be no difference 
between
   
   ```c++
   ServiceUrlProvider provider = /* ... */;
   Client client(provider());
   ```
   
   and
   
   ```c++
   ServiceUrlProvider provider = /* ... */;
   String serviceUrl = provider();
   Client client(serviceUrl);
   ```
   
   l'm going to send an email to dev mail list to discuss.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to