[GitHub] [flink] reswqa commented on a diff in pull request #22438: [FLINK-31873] Add method setMaxParallelism to DataStreamSink.

["reswqa (via GitHub)" <gi...@apache.org>]

reswqa commented on code in PR #22438:
URL: https://github.com/apache/flink/pull/22438#discussion_r1178591601


##########
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/graph/StreamingJobGraphGeneratorTest.java:
##########
@@ -283,6 +283,27 @@ public void testTransformationSetParallelism() {
         assertThat(vertices.get(2).isParallelismConfigured()).isTrue();
     }
 
+    @Test
+    void testTransformationSetMaxParallelism() {
+        StreamExecutionEnvironment env = StreamExecutionEnvironment.getExecutionEnvironment();
+        /* The max parallelism of the environment (that is inherited by the source)
+        and the parallelism of the map operator needs to be different for this test */

Review Comment:
   In Flink, `//` is generally used directly for comments.



##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/datastream/DataStreamSink.java:
##########
@@ -180,6 +180,18 @@ public DataStreamSink<T> setParallelism(int parallelism) {
         return this;
     }
 
+    /**
+     * Sets the max parallelism for this sink. The degree must be higher than zero and less than the
+     * upper bound.

Review Comment:
   > The degree must be higher than zero and less than the upper bound.
   
   Just stating this in java doc is not safe enough, we'd better have directly sanity check for this. For example:
   `OperatorValidationUtils.validateMaxParallelism(maxParallelism, yes);`



##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/datastream/DataStreamSink.java:
##########
@@ -180,6 +180,18 @@ public DataStreamSink<T> setParallelism(int parallelism) {
         return this;
     }
 
+    /**
+     * Sets the max parallelism for this sink. The degree must be higher than zero and less than the
+     * upper bound.

Review Comment:
   > The degree must be higher than zero and less than the upper bound.
   
   Just stating this in java doc is not safe enough, we'd better have directly sanity check for this. For example:
   `OperatorValidationUtils.validateMaxParallelism(maxParallelism, true);`



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org