lhotari commented on code in PR #23989:
URL: https://github.com/apache/pulsar/pull/23989#discussion_r2420938434
##########
pulsar-client/src/test/java/org/apache/pulsar/client/impl/ClientBuilderImplTest.java:
##########
@@ -174,7 +174,7 @@ public void testLoadConfAuthNotSetWhenNoPropsAvailable() {
@Test(expectedExceptions = IllegalArgumentException.class)
public void testClientBuilderWithMaxRetryTime() throws
PulsarClientException {
- PulsarClient.builder().maxRetryTimes(5).build();
+ PulsarClient.builder().MaxHttpRequestRetries(5).build();
Review Comment:
```suggestion
PulsarClient.builder().maxHttpRequestRetries(5).build();
```
##########
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java:
##########
@@ -721,10 +721,10 @@ ClientBuilder authentication(String authPluginClassName,
Map<String, String> aut
/**
* Set the max retry times for a request.
*
- * @param maxRetryTimes
+ * @param maxHttpRequestRetries
* @return the client builder instance
*/
- ClientBuilder maxRetryTimes(int maxRetryTimes);
+ ClientBuilder MaxHttpRequestRetries(int maxHttpRequestRetries);
Review Comment:
```suggestion
ClientBuilder maxHttpRequestRetries(int maxHttpRequestRetries);
```
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientBuilderImpl.java:
##########
@@ -509,8 +509,8 @@ public ClientBuilder lookupProperties(Map<String, String>
properties) {
}
@Override
- public ClientBuilder maxRetryTimes(int maxRetryTimes) {
- conf.setMaxRetryTimes(maxRetryTimes);
+ public ClientBuilder MaxHttpRequestRetries(int maxHttpRequestRetries) {
Review Comment:
```suggestion
public ClientBuilder maxHttpRequestRetries(int maxHttpRequestRetries) {
```
##########
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java:
##########
@@ -718,6 +718,13 @@ ClientBuilder authentication(String authPluginClassName,
Map<String, String> aut
*/
ClientBuilder lookupProperties(Map<String, String> properties);
+ /**
+ * Set the max retry times for a request.
+ *
+ * @param maxRetryTimes
+ * @return the client builder instance
+ */
+ ClientBuilder maxRetryTimes(int maxRetryTimes);
Review Comment:
The Pulsar Admin Builder implementation also uses the
ClientConfigurationData to hold configuration data (which isn't the greatest
solution since not all configuration applies to both ClientBuilder and
PulsarAdminBuilder).
##########
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java:
##########
@@ -718,6 +718,13 @@ ClientBuilder authentication(String authPluginClassName,
Map<String, String> aut
*/
ClientBuilder lookupProperties(Map<String, String> properties);
+ /**
+ * Set the max retry times for a request.
+ *
+ * @param maxRetryTimes
+ * @return the client builder instance
+ */
+ ClientBuilder maxRetryTimes(int maxRetryTimes);
Review Comment:
ClientBuilder isn't used for PulsarAdmin client. It's the
org.apache.pulsar.client.admin.PulsarAdminBuilder that is used. I'd expect this
setting to be in PulsarAdminBuilder instead of ClientBuilder, however there's
also a Http client in the Pulsar client itself which is used when the provided
serviceUrl is a http/https url.
##########
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java:
##########
@@ -721,10 +721,10 @@ ClientBuilder authentication(String authPluginClassName,
Map<String, String> aut
/**
* Set the max retry times for a request.
Review Comment:
```suggestion
* Set the maximum number of retries for http requests.
```
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ClientConfigurationData.java:
##########
Review Comment:
update the name
--
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]
