Inconsistent Use of executionTimeout Parameter in Client Component
Description
In the Client component, the executionTimeout parameter is used with different time units, leading to confusion during its usage.
-
When setting the value: The
executionTimeoutparameter is interpreted in milliseconds. - When retrieving the value: The parameter is returned in seconds.
Methods with Inconsistent Usage:
-
setExecutionTimeout(long timeout, ChronoUnit timeUnit):
- This method sets the
executionTimeoutin milliseconds.
public Builder setExecutionTimeout(long timeout, ChronoUnit timeUnit) { this.configuration.put(ClientConfigProperties.MAX_EXECUTION_TIME.getKey(), String.valueOf(Duration.of(timeout, timeUnit).toMillis())); return this; } - This method sets the
-
getOperationTimeout():
- This method retrieves the
executionTimeoutbut does not clarify the unit of measurement in its JavaDoc, leading to confusion as it returns the value as an integer.
protected int getOperationTimeout() { return Integer.parseInt((String)this.configuration.get(ClientConfigProperties.MAX_EXECUTION_TIME.getKey())); } - This method retrieves the
-
Usage in queryAll and getTableSchemaImpl:
- In
queryAll, the timeout is used in milliseconds:
try (QueryResponse response = operationTimeout == 0 ? query(sqlQuery, settings).get() : query(sqlQuery, settings).get(operationTimeout, TimeUnit.MILLISECONDS)) { ... }- In
getTableSchemaImpl, the timeout is used in seconds:
try (QueryResponse response = operationTimeout == 0 ? query(describeQuery, queryParams, settings).get() : query(describeQuery, queryParams, settings).get(getOperationTimeout(), TimeUnit.SECONDS)) { ... } - In
JavaDoc Issue: The JavaDoc for this method states that the returned value is in seconds, which is inconsistent with how the value is set.
Expected Behavior:
The units of measurement should be consistent. It is recommended to use the same unit of measurement (either milliseconds or seconds) for both setting and retrieving the value. Additionally, the JavaDoc for the getOperationTimeout() method should be updated to reflect the correct unit of measurement.
Steps to Reproduce:
- Set the
executionTimeoutvalue in the Client component (e.g.,1000). - Retrieve the
executionTimeoutvalue. - Note the discrepancy in units of measurement.
Additional Information: This inconsistency may lead to errors and misunderstandings on the part of users of the component. The JavaDoc for the method that retrieves this value should be corrected to indicate the correct unit of measurement.
Environment
- Client-v2 version: 0.8.5