gemini-code-assist[bot] commented on code in PR #38960:
URL: https://github.com/apache/beam/pull/38960#discussion_r3412700016
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowWorkerLoggingOptions.java:
##########
@@ -30,13 +30,11 @@
/**
* Options that are used to control logging configuration on the Dataflow
worker.
*
- * @deprecated This interface will no longer be the source of truth for worker
logging configuration
- * once jobs are executed using a dedicated SDK harness instead of user
code being co-located
- * alongside Dataflow worker code. Consider set corresponding options
within {@link
- * org.apache.beam.sdk.options.SdkHarnessOptions} to ensure forward
compatibility.
+ * <p> Some options in this interface are no longer the source of truth for
worker logging configuration
+ * Consider using the corresponding options within {@link
org.apache.beam.sdk.options.SdkHarnessOptions} to ensure
+ * compatibility with other runners.
Review Comment:
Add a missing period at the end of the sentence and remove the leading space
after `<p>` for standard Javadoc formatting.
```suggestion
* <p>Some options in this interface are no longer the source of truth for
worker logging configuration.
* Consider using the corresponding options within {@link
org.apache.beam.sdk.options.SdkHarnessOptions} to ensure
* compatibility with other runners.
```
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowWorkerLoggingOptions.java:
##########
@@ -59,9 +57,14 @@ enum Level {
TRACE
}
- /** This option controls the default log level of all loggers without a log
level override. */
+ /** This option controls the default log level of all loggers without a log
level override.
+ *
+ * @deprecated Prefer defaultSdkHarnessLogLevel within
org.apache.beam.sdk.options.SdkHarnessOptions
+ * which works across runners.
+ * */
@Description("Controls the default log level of all loggers without a log
level override.")
@Default.Enum("INFO")
+ @Deprecated
Review Comment:
When deprecating a property in a `PipelineOptions` interface, both the
getter and the setter should be deprecated to ensure consistent compiler
warnings. Please also add `@Deprecated` to `setDefaultWorkerLogLevel(Level
level)`.
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowWorkerLoggingOptions.java:
##########
@@ -120,12 +127,16 @@ enum Level {
* <p>Note that the message may be filtered depending on the {@link
#getDefaultWorkerLogLevel
* defaultWorkerLogLevel} or if a {@code System.err} override is specified
via {@link
* #getWorkerLogLevelOverrides workerLogLevelOverrides}.
+ *
+ * @deprecated Prefer using
org.apache.beam.sdk.options.SdkHarnessOptions.sdkHarnessLogLevelOverrides to
override the
+ * 'System.err' logger as this works across runners.
Review Comment:
Use a proper Javadoc `{@link}` tag to reference the replacement option.
```suggestion
* @deprecated Prefer using {@link
org.apache.beam.sdk.options.SdkHarnessOptions#getSdkHarnessLogLevelOverrides()}
to override the
* 'System.err' logger as this works across runners.
```
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowWorkerLoggingOptions.java:
##########
@@ -149,6 +163,7 @@ enum Level {
+ "level. System.out and System.err levels are configured via
loggers of the corresponding "
+ "name. Also, note that when multiple overrides are specified, the
exact name followed by "
+ "the closest parent takes precedence.")
+ @Deprecated
Review Comment:
Please also deprecate the corresponding setter
`setWorkerLogLevelOverrides(WorkerLogLevelOverrides value)` with `@Deprecated`
to ensure consistency.
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowWorkerLoggingOptions.java:
##########
@@ -59,9 +57,14 @@ enum Level {
TRACE
}
- /** This option controls the default log level of all loggers without a log
level override. */
+ /** This option controls the default log level of all loggers without a log
level override.
+ *
+ * @deprecated Prefer defaultSdkHarnessLogLevel within
org.apache.beam.sdk.options.SdkHarnessOptions
+ * which works across runners.
+ * */
Review Comment:
Use a proper Javadoc `{@link}` tag to reference the replacement option. This
makes it easier for developers to navigate to the new option in their IDEs.
```suggestion
/**
* This option controls the default log level of all loggers without a log
level override.
*
* @deprecated Prefer {@link
org.apache.beam.sdk.options.SdkHarnessOptions#getDefaultSdkHarnessLogLevel()}
* which works across runners.
*/
```
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowWorkerLoggingOptions.java:
##########
@@ -104,12 +107,16 @@ enum Level {
* <p>Note that the message may be filtered depending on the {@link
#getDefaultWorkerLogLevel
* defaultWorkerLogLevel} or if a {@code System.out} override is specified
via {@link
* #getWorkerLogLevelOverrides workerLogLevelOverrides}.
+ *
+ * @deprecated Prefer using
org.apache.beam.sdk.options.SdkHarnessOptions.sdkHarnessLogLevelOverrides to
override the
+ * 'System.out' logger as this works across runners.
Review Comment:
Use a proper Javadoc `{@link}` tag to reference the replacement option.
```suggestion
* @deprecated Prefer using {@link
org.apache.beam.sdk.options.SdkHarnessOptions#getSdkHarnessLogLevelOverrides()}
to override the
* 'System.out' logger as this works across runners.
```
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowWorkerLoggingOptions.java:
##########
@@ -138,6 +149,9 @@ enum Level {
* <p>See {@link WorkerLogLevelOverrides} for more information on how to
configure logging on a
* per {@link Class}, {@link Package}, or name basis. If used from the
command line, the expected
* format is {"Name":"Level",...}, further details on {@link
WorkerLogLevelOverrides#from}.
+ *
+ * @deprecated Prefer using
org.apache.beam.sdk.options.SdkHarnessOptions.sdkHarnessLogLevelOverrides which
works
+ * across runners.
Review Comment:
Use a proper Javadoc `{@link}` tag to reference the replacement option.
```suggestion
* @deprecated Prefer using {@link
org.apache.beam.sdk.options.SdkHarnessOptions#getSdkHarnessLogLevelOverrides()}
which works
* across runners.
```
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowWorkerLoggingOptions.java:
##########
@@ -104,12 +107,16 @@ enum Level {
* <p>Note that the message may be filtered depending on the {@link
#getDefaultWorkerLogLevel
* defaultWorkerLogLevel} or if a {@code System.out} override is specified
via {@link
* #getWorkerLogLevelOverrides workerLogLevelOverrides}.
+ *
+ * @deprecated Prefer using
org.apache.beam.sdk.options.SdkHarnessOptions.sdkHarnessLogLevelOverrides to
override the
+ * 'System.out' logger as this works across runners.
*/
@Description(
"Controls the log level given to messages printed to System.out. Note
that the "
+ "message may be filtered depending on the defaultWorkerLogLevel or
if a 'System.out' "
+ "override is specified via workerLogLevelOverrides.")
@Default.Enum("INFO")
+ @Deprecated
Review Comment:
Please also deprecate the corresponding setter
`setWorkerSystemOutMessageLevel(Level level)` with `@Deprecated` to ensure
consistency.
##########
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/options/DataflowWorkerLoggingOptions.java:
##########
@@ -120,12 +127,16 @@ enum Level {
* <p>Note that the message may be filtered depending on the {@link
#getDefaultWorkerLogLevel
* defaultWorkerLogLevel} or if a {@code System.err} override is specified
via {@link
* #getWorkerLogLevelOverrides workerLogLevelOverrides}.
+ *
+ * @deprecated Prefer using
org.apache.beam.sdk.options.SdkHarnessOptions.sdkHarnessLogLevelOverrides to
override the
+ * 'System.err' logger as this works across runners.
*/
@Description(
"Controls the log level given to messages printed to System.err. Note
that the "
+ "message may be filtered depending on the defaultWorkerLogLevel or
if a 'System.err' "
+ "override is specified via workerLogLevelOverrides.")
@Default.Enum("ERROR")
+ @Deprecated
Review Comment:
Please also deprecate the corresponding setter
`setWorkerSystemErrMessageLevel(Level level)` with `@Deprecated` to ensure
consistency.
--
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]
