scwhittle commented on code in PR #31091:
URL: https://github.com/apache/beam/pull/31091#discussion_r1579521911
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowPipelineDebugOptions.java:
##########
@@ -216,14 +216,27 @@ public Dataflow create(PipelineOptions options) {
void setReaderCacheTimeoutSec(Integer value);
- /** The max amount of time an UnboundedReader is consumed before
checkpointing. */
+ /**
+ * The max amount of time an UnboundedReader is consumed before
checkpointing.
+ *
+ * @deprecated use {@link
DataflowPipelineDebugOptions#getUnboundedReaderMaxReadTimeMs()} instead
+ */
@Description(
"The max amount of time before an UnboundedReader is consumed before
checkpointing, in seconds.")
@Default.Integer(10)
Review Comment:
can the default be removed here? I think it will be less brittle since the
default won't be repeated in the WorkerCustomSources
if (maxReadTimeMsDeprecated != null and maxReadTimeMsDeprecated > 0) {
// use it. Seems fine just pick if both were set.
} else {
use new option, either the default or explicitly set
}
or another option could be to use a DefaultFactory (see StagerFactory in
this file) so that the default of the new field is to be 1000 *
getUnboundedReaderMaxReadTimeSec
Then the code could just use getUnboundedReaderMaxReadTimeMs() and it would
either default to fallback to previous value (set or default) or prefer the new
value if it was set.
--
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]
