flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] azagrebin commented on a change in pull request #9910: [FLINK-14405][runtime] Align ResourceProfile/ResourceSpec fields with the new TaskExecutor memory setups.
Date Wed, 16 Oct 2019 15:36:36 GMT
azagrebin commented on a change in pull request #9910: [FLINK-14405][runtime] Align ResourceProfile/ResourceSpec
fields with the new TaskExecutor memory setups.
URL: https://github.com/apache/flink/pull/9910#discussion_r335515846
 
 

 ##########
 File path: flink-core/src/main/java/org/apache/flink/api/common/operators/ResourceSpec.java
 ##########
 @@ -310,50 +327,73 @@ private Object readResolve() {
 	//  builder
 	// ------------------------------------------------------------------------
 
-	public static Builder newBuilder() {
-		return new Builder();
+	public static Builder newBuilder(double cpuCores, MemorySize taskHeapMemory) {
+		return new Builder(cpuCores, taskHeapMemory);
+	}
+
+	public static Builder newBuilder(double cpuCores, int taskHeapMemoryMB) {
 
 Review comment:
   This builder introduces quite a lot of unrelated changes. I would move them at least to
a separate hotfix commit to simplify review as well.
   
   In general, although this minor refactoring reduces the code, it again changes the git
history which is arguable because it makes a move back to unnamed method parameters and builder
is usually added to name the parameters for readability. All-in-all I would be in favour of
not touching it as it is not necessary.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message