damccorm commented on code in PR #39194:
URL: https://github.com/apache/beam/pull/39194#discussion_r3537577165
##########
sdks/java/ml/inference/remote/src/main/java/org/apache/beam/sdk/ml/inference/remote/RemoteInference.java:
##########
@@ -164,6 +214,13 @@ public void setupHandler() {
try {
this.modelHandler =
handlerClass.getDeclaredConstructor().newInstance();
this.modelHandler.createClient(parameters);
+ this.throttler =
Review Comment:
Alternatively, we could keep it nullable and allow a way for disabling it
(e.g. set `throttleDelaySecs` to 0)
##########
sdks/java/ml/inference/remote/src/main/java/org/apache/beam/sdk/ml/inference/remote/RemoteInference.java:
##########
@@ -164,6 +214,13 @@ public void setupHandler() {
try {
this.modelHandler =
handlerClass.getDeclaredConstructor().newInstance();
this.modelHandler.createClient(parameters);
+ this.throttler =
Review Comment:
Should we instantiate this in the constructor and make it non-nullable?
##########
sdks/java/ml/inference/remote/src/main/java/org/apache/beam/sdk/ml/inference/remote/RemoteInference.java:
##########
@@ -113,6 +130,30 @@ public Invoke<InputT, OutputT>
withBatchConfig(BatchElements.BatchConfig batchCo
return builder().setBatchConfig(batchConfig).build();
}
+ /** Configures the throttling delay when the client is preemptively
throttled. */
Review Comment:
Also, I don't know if these doc strings are sufficient to understand the
feature. Maybe we could link to the throttling overview doc (either design doc
or the Google SWE book page)
##########
sdks/java/ml/inference/remote/src/main/java/org/apache/beam/sdk/ml/inference/remote/RemoteInference.java:
##########
@@ -113,6 +130,30 @@ public Invoke<InputT, OutputT>
withBatchConfig(BatchElements.BatchConfig batchCo
return builder().setBatchConfig(batchConfig).build();
}
+ /** Configures the throttling delay when the client is preemptively
throttled. */
Review Comment:
Should we document the default values for these parameters here?
--
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]
