[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r221169102
 
 

 ##
 File path: 
flink-core/src/main/java/org/apache/flink/api/common/ExecutionConfig.java
 ##
 @@ -103,6 +103,8 @@
 */
private int maxParallelism = -1;
 
+   private String description;
 
 Review comment:
   This is not needed here.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r221169745
 
 

 ##
 File path: 
flink-optimizer/src/main/java/org/apache/flink/optimizer/traversals/PlanFinalizer.java
 ##
 @@ -76,7 +76,7 @@ public PlanFinalizer() {
this.stackOfIterationNodes = new 
ArrayDeque();
}
 
-   public OptimizedPlan createFinalPlan(List sinks, String 
jobName, Plan originalPlan) {
+   public OptimizedPlan createFinalPlan(List sinks, String 
jobName, Plan originalPlan, String jobDescription) {
 
 Review comment:
   `jobName` and `jobDescription` are not needed anymore.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r221169522
 
 

 ##
 File path: 
flink-optimizer/src/main/java/org/apache/flink/optimizer/plan/OptimizedPlan.java
 ##
 @@ -46,25 +46,20 @@
/** The original program (as a dataflow plan). */
private final Plan originalProgram;
 
-   /** Name of the job */
-   private final String jobName;
-
/**
 * Creates a new instance of this optimizer plan container. The plan is 
given and fully
 * described by the data sources, sinks and the collection of all nodes.
-* 
+*
 * @param sources The data sources.
 * @param sinks The data sinks.
 * @param allNodes A collection containing all nodes in the plan.
-* @param jobName The name of the program
 */
public OptimizedPlan(Collection sources, 
Collection sinks,
-   Collection allNodes, String jobName, Plan 
programPlan)
+Collection allNodes, 
Plan programPlan)
 
 Review comment:
   Please revert the indentation.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r221169255
 
 

 ##
 File path: flink-core/src/main/java/org/apache/flink/api/common/Plan.java
 ##
 @@ -231,6 +239,24 @@ public void setJobName(String jobName) {
this.jobName = jobName;
}
 
+   /**
+* Gets the description of this plan.
+*
+* @return The description of the plan.
+*/
+   public String getJobDescription() {
+   return jobDescription;
+   }
+
+   /**
+* Sets the jobDescription for this Plan.
+*
+* @param jobDescription The jobDescription to set.
+*/
+   public void setJobDescription(String jobDescription) {
+   this.jobDescription = jobDescription;
 
 Review comment:
   `checkNotNull`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219449333
 
 

 ##
 File path: 
flink-java/src/main/java/org/apache/flink/api/java/ExecutionEnvironment.java
 ##
 @@ -127,6 +127,8 @@
/** Flag to indicate whether sinks have been cleared in previous 
executions. */
private boolean wasExecuted = false;
 
+   private String description;
 
 Review comment:
   I think `ExecutionEnvironment` should not have the description. I would 
really prefer to follow exactly the same scenario as with `jobName`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219486186
 
 

 ##
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/messages/webmonitor/JobDetails.java
 ##
 @@ -97,6 +101,7 @@ public JobDetails(
"tasksPerState argument must be of size {}.", 
ExecutionState.values().length);
this.tasksPerState = checkNotNull(tasksPerState);
this.numTasks = numTasks;
+   this.jobDescription = jobDescription;
 
 Review comment:
   Please check with `checkNotNull`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219476983
 
 

 ##
 File path: 
flink-optimizer/src/main/java/org/apache/flink/optimizer/plan/OptimizedPlan.java
 ##
 @@ -49,23 +49,28 @@
/** Name of the job */
private final String jobName;
 
+   /** Descrption of the job */
+   private final String jobDescription;
+
/**
 * Creates a new instance of this optimizer plan container. The plan is 
given and fully
 * described by the data sources, sinks and the collection of all nodes.
-* 
+*
 * @param sources The data sources.
 * @param sinks The data sinks.
 * @param allNodes A collection containing all nodes in the plan.
 * @param jobName The name of the program
+* @param jobDescription The description of the program
 */
public OptimizedPlan(Collection sources, 
Collection sinks,
-   Collection allNodes, String jobName, Plan 
programPlan)
+Collection allNodes, 
String jobName, Plan programPlan, String jobDescription)
 
 Review comment:
   Actually I would remove the `jobName` and `jobDescription` parameters, and 
use those values from the programPlan, which we always do anyways.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219485200
 
 

 ##
 File path: 
flink-runtime/src/test/java/org/apache/flink/runtime/messages/WebMonitorMessagesTest.java
 ##
 @@ -147,7 +147,7 @@ public void testMultipleJobDetails() {
JobID jid = new JobID();
JobStatus status = 
JobStatus.values()[rnd.nextInt(JobStatus.values().length)];
 
-   details[k] = new JobDetails(jid, name, time, endTime, 
endTime - time, status, lastModified, numVerticesPerState, numTotal);
+   details[k] = new JobDetails(jid, name, time, endTime, 
endTime - time, status, lastModified, numVerticesPerState, numTotal, "");
 
 Review comment:
   Please use the `GenericMessageTester` for the description the same way it is 
used for `jobName`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219477941
 
 

 ##
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java
 ##
 @@ -198,6 +201,24 @@ public String getName() {
return this.jobName;
}
 
+   /**
+* Returns the description assigned to the job graph.
+*
+* @return the description assigned to the job graph
+*/
+   public String getDescription() {
+   return jobDescription;
+   }
+
+   /**
+* Set the description for this job graph.
+*
+* @param jobDescription the description of the job graph
+*/
+   public void setDescription(String jobDescription) {
 
 Review comment:
   Please set the `jobDescription` only in the ctor.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219486325
 
 

 ##
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/JobInformation.java
 ##
 @@ -61,13 +64,15 @@ public JobInformation(
SerializedValue 
serializedExecutionConfig,
Configuration jobConfiguration,
Collection requiredJarFileBlobKeys,
-   Collection requiredClasspathURLs) {
+   Collection requiredClasspathURLs,
+   String jobDescription) {
this.jobId = Preconditions.checkNotNull(jobId);
this.jobName = Preconditions.checkNotNull(jobName);
this.serializedExecutionConfig = 
Preconditions.checkNotNull(serializedExecutionConfig);
this.jobConfiguration = 
Preconditions.checkNotNull(jobConfiguration);
this.requiredJarFileBlobKeys = 
Preconditions.checkNotNull(requiredJarFileBlobKeys);
this.requiredClasspathURLs = 
Preconditions.checkNotNull(requiredClasspathURLs);
+   this.jobDescription = jobDescription;
 
 Review comment:
   Please check with `checkNotNull`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219517956
 
 

 ##
 File path: 
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/StreamExecutionEnvironment.scala
 ##
 @@ -56,6 +56,22 @@ class StreamExecutionEnvironment(javaEnv: JavaEnv) {
 */
   def getCachedFiles = javaEnv.getCachedFiles
 
+  /**
+* Gets the description of the job.
+*
+* @return the job's description
+*/
+  def getDescription: String = javaEnv.getDescription
 
 Review comment:
   I would remove those methods, as described above.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219485512
 
 

 ##
 File path: 
flink-runtime/src/test/java/org/apache/flink/runtime/rest/handler/legacy/utils/ArchivedExecutionGraphBuilder.java
 ##
 @@ -55,6 +55,7 @@
private ArchivedExecutionConfig archivedExecutionConfig;
private boolean isStoppable;
private Map>> 
serializedUserAccumulators;
+   private String jobDescription;
 
 Review comment:
   Please move it next to `jobName`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219485130
 
 

 ##
 File path: 
flink-runtime/src/test/java/org/apache/flink/runtime/messages/WebMonitorMessagesTest.java
 ##
 @@ -92,8 +92,8 @@ public void testJobDetailsMessage() {
JobID jid = GenericMessageTester.randomJobId(rnd);
JobStatus status = 
GenericMessageTester.randomJobStatus(rnd);

-   JobDetails msg1 = new JobDetails(jid, name, time, 
endTime, endTime - time, status, lastModified, numVerticesPerState, numTotal);
-   JobDetails msg2 = new JobDetails(jid, name, time, 
endTime, endTime - time, status, lastModified, numVerticesPerState, numTotal);
+   JobDetails msg1 = new JobDetails(jid, name, time, 
endTime, endTime - time, status, lastModified, numVerticesPerState, numTotal, 
"");
 
 Review comment:
   Please use the `GenericMessageTester` for the description the same way it is 
used for `jobName`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r219487366
 
 

 ##
 File path: 
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/StreamExecutionEnvironment.java
 ##
 @@ -156,6 +158,25 @@ public ExecutionConfig getConfig() {
return config;
}
 
+   /**
+* Gets the description of the job.
+*
+* @return the job's description
+*/
+   public String getDescription() {
 
 Review comment:
   I would prefer not to add those methods. I would handle the description the 
same way the `jobName` is.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r216248829
 
 

 ##
 File path: 
flink-core/src/main/java/org/apache/flink/api/common/ExecutionConfig.java
 ##
 @@ -103,6 +103,8 @@
 */
private int maxParallelism = -1;
 
+   private String description;
 
 Review comment:
   If you implemented it the same way as jobName, you would have it there in 
the `OptimizedPlan`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dawidwys commented on a change in pull request #6266: [FLINK-9682] Add setDescription to execution environment and provide description field for the rest api

[GitBox]

dawidwys commented on a change in pull request #6266: [FLINK-9682] Add 
setDescription to execution environment and provide description field for the 
rest api
URL: https://github.com/apache/flink/pull/6266#discussion_r216226971
 
 

 ##
 File path: 
flink-core/src/main/java/org/apache/flink/api/common/ExecutionConfig.java
 ##
 @@ -103,6 +103,8 @@
 */
private int maxParallelism = -1;
 
+   private String description;
 
 Review comment:
   Why is it in the `ExecutionConfig`? I think it doesn't fit in here.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services