Fix Flow/Job ID/Url mixups (linkedin#100)

anspuli · Jul 22, 2016 · 84919e2 · 84919e2
1 parent a8b2f8a
commit 84919e2
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 19 deletions.
diff --git a/app/controllers/Application.java b/app/controllers/Application.java
@@ -154,7 +154,11 @@ public static Result dashboard() {
 
   /**
    * Returns the scheduler info id/url pair for the most recent app result that has an id like value
-   * (which can use % and _ SQL wild cards) for the specified field.
+   * (which can use % and _ SQL wild cards) for the specified field. Note that this is a pair rather
+   * than merely an ID/URL because for some schedulers (e.g. Airflow) they are not equivalent and
+   * usually the UI wants to display the ID with a link to the URL. While it is true that the URL
+   * can probably be derived from the ID in most cases, we would need scheduler specific logic which
+   * would be a mess.
    */
   private static IdUrlPair bestSchedulerInfoMatchLikeValue(String value, String schedulerIdField) {
     String schedulerUrlField;
@@ -239,7 +243,7 @@ public static Result search() {
           .eq(AppResult.TABLE.FLOW_EXEC_ID, flowExecPair.getId())
           .findList();
       Map<IdUrlPair, List<AppResult>> map = groupJobs(results, GroupBy.JOB_EXECUTION_ID);
-      return ok(searchPage.render(null, flowDetails.render(flowExecPair.getId(), map)));
+      return ok(searchPage.render(null, flowDetails.render(flowExecPair, map)));
     }
 
     // Prepare pagination of results
@@ -566,14 +570,14 @@ public static Result flowHistory() {
 
     if (graphType.equals("heuristics")) {
       return ok(flowHistoryPage.render(flowDefPair.getId(), graphType,
-          flowHistoryResults.render(flowDefPair.getId(), executionMap, idPairToJobNameMap, flowExecTimeList)));
+          flowHistoryResults.render(flowDefPair, executionMap, idPairToJobNameMap, flowExecTimeList)));
     } else if (graphType.equals("resources") || graphType.equals("time")) {
       if (hasSparkJob) {
         return notFound("Cannot plot graph for " + graphType + " since it contains a spark job. " + graphType
             + " graphs are not supported for spark right now");
       } else {
         return ok(flowHistoryPage.render(flowDefPair.getId(), graphType,
-            flowMetricsHistoryResults.render(flowDefPair.getId(), graphType, executionMap, idPairToJobNameMap,
+            flowMetricsHistoryResults.render(flowDefPair, graphType, executionMap, idPairToJobNameMap,
                 flowExecTimeList)));
       }
     }
@@ -668,13 +672,13 @@ public static Result jobHistory() {
 
     if (graphType.equals("heuristics")) {
       return ok(jobHistoryPage.render(jobDefPair.getId(), graphType,
-          jobHistoryResults.render(jobDefPair.getId(), executionMap, maxStages, flowExecTimeList)));
+          jobHistoryResults.render(jobDefPair, executionMap, maxStages, flowExecTimeList)));
     } else if (graphType.equals("resources") || graphType.equals("time")) {
       if (hasSparkJob) {
         return notFound("Resource and time graph are not supported for spark right now");
       } else {
         return ok(jobHistoryPage.render(jobDefPair.getId(), graphType,
-            jobMetricsHistoryResults.render(jobDefPair.getId(), graphType, executionMap, maxStages, flowExecTimeList)));
+            jobMetricsHistoryResults.render(jobDefPair, graphType, executionMap, maxStages, flowExecTimeList)));
       }
     }
 

diff --git a/app/views/results/flowDetails.scala.html b/app/views/results/flowDetails.scala.html
@@ -14,20 +14,20 @@
 * the License.
 *@
 
-@(execurl: String, results: java.util.Map[IdUrlPair, java.util.List[models.AppResult]])
+@(flowExecPair: IdUrlPair, results: java.util.Map[IdUrlPair, java.util.List[models.AppResult]])
 
 @*
 * Displays all the mr jobs belonging to a flow grouped by job exec url
 *
-* @param execurl The Flow execution URL
+* @param flowExecPair The flow execution pair
 * @param results A map from job Exec URL to all the MR jobs.
 *@
 
 <div class="panel panel-default">
 
   <div class="panel-heading">
     <h3 class="panel-title">
-      Flow Execution URL: <a href=@execurl>@execurl</a>
+      Flow Execution URL: <a href=@flowExecPair.getUrl>@flowExecPair.getId</a>
     </h3>
   </div>
 

diff --git a/app/views/results/flowHistoryResults.scala.html b/app/views/results/flowHistoryResults.scala.html
@@ -14,7 +14,7 @@
 * the License.
 *@
 
-@(flowDefId: String, results: java.util.Map[IdUrlPair, java.util.Map[IdUrlPair, java.util.List[models.AppResult]]],
+@(flowDefPair: IdUrlPair, results: java.util.Map[IdUrlPair, java.util.Map[IdUrlPair, java.util.List[models.AppResult]]],
     idPairToJobNameMap: java.util.Map[IdUrlPair, String], flowExecTimeList: java.util.List[Long])
 
 @import com.linkedin.drelephant.analysis.Severity
@@ -35,7 +35,7 @@
 }
 
 @if(results != null && results.nonEmpty) {
-  @tags.panel(){ Flow History Results: <a href="@flowDefId" style="font-size:14px; color:#083d8d">@flowDefId</a>} {
+  @tags.panel(){ Flow History Results: <a href="@flowDefPair.getUrl" style="font-size:14px; color:#083d8d">@flowDefPair.getId</a>} {
 
     <script src="@routes.Assets.at("js/flowhistoryform.js")" type="text/javascript"></script>
     <script src="@routes.Assets.at("js/graphutility.js")" type="text/javascript"></script>
@@ -59,8 +59,8 @@
               <th style="width:200px">Flow Executions</th>
               @for((jobDefPair, jobName) <- idPairToJobNameMap) {
                 <th>
-                  <a href='/[email protected](jobDefPair.getUrl)&select-graph-type=heuristics' data-toggle='tooltip'
-                  title='@jobDefPair.getUrl'>Job @{jobDefIndex = jobDefIndex + 1; jobDefIndex}<br>
+                  <a href='/[email protected](jobDefPair.getId)&select-graph-type=heuristics' data-toggle='tooltip'
+                  title='@jobDefPair.getId'>Job @{jobDefIndex = jobDefIndex + 1; jobDefIndex}<br>
                   @if(jobName.length > 45) { @jobName.substring(0, 41)... } else { @jobName }
                   </a>
                 </th>

diff --git a/app/views/results/flowMetricsHistoryResults.scala.html b/app/views/results/flowMetricsHistoryResults.scala.html
@@ -14,7 +14,7 @@
 * the License.
 *@
 
-@(flowDefId: String, graphType: String, results: java.util.Map[IdUrlPair, java.util.Map[IdUrlPair, java.util.List[models.AppResult]]],
+@(flowDefPair: IdUrlPair, graphType: String, results: java.util.Map[IdUrlPair, java.util.Map[IdUrlPair, java.util.List[models.AppResult]]],
         idPairToJobNameMap: java.util.Map[IdUrlPair, String], flowExecTimeList: java.util.List[Long])
 
 @import com.linkedin.drelephant.util.Utils;
@@ -36,7 +36,7 @@
 }
 
 @if(results != null && results.nonEmpty) {
-  @tags.panel(){ Flow History Results: <a href="@flowDefId" style="font-size:14px; color:#083d8d">@flowDefId</a>} {
+  @tags.panel(){ Flow History Results: <a href="@flowDefPair.getUrl" style="font-size:14px; color:#083d8d">@flowDefPair.getId</a>} {
 
     @if(graphType.equals("resources")) {
       <script src="@routes.Assets.at("js/flowresourcehistoryform.js")" type="text/javascript"></script>

diff --git a/app/views/results/jobHistoryResults.scala.html b/app/views/results/jobHistoryResults.scala.html
@@ -14,7 +14,7 @@
 * the License.
 *@
 
-@(jobDefId: String, results: java.util.Map[IdUrlPair, java.util.List[models.AppResult]], maxStages: Int,
+@(jobDefPair: IdUrlPair, results: java.util.Map[IdUrlPair, java.util.List[models.AppResult]], maxStages: Int,
     flowExecTimeList:java.util.List[Long])
 
 @import com.linkedin.drelephant.analysis.Severity
@@ -34,7 +34,7 @@
 }
 
 @if(results != null && results.nonEmpty) {
-  @tags.panel(){ Job History Results: <a href="@jobDefId" style="font-size:14px; color:#083d8d">@jobDefId</a>} {
+  @tags.panel(){ Job History Results: <a href="@jobDefPair.getUrl" style="font-size:14px; color:#083d8d">@jobDefPair.getId</a>} {
     <script src="@routes.Assets.at("js/jobhistoryform.js")" type="text/javascript"></script>
     <script src="@routes.Assets.at("js/graphutility.js")" type="text/javascript"></script>
 

diff --git a/app/views/results/jobMetricsHistoryResults.scala.html b/app/views/results/jobMetricsHistoryResults.scala.html
@@ -14,7 +14,7 @@
 * the License.
 *@
 
-@(jobDefId: String, graphType: String, results: java.util.Map[IdUrlPair, java.util.List[models.AppResult]], maxStages: Int,
+@(jobDefPair: IdUrlPair, graphType: String, results: java.util.Map[IdUrlPair, java.util.List[models.AppResult]], maxStages: Int,
         flowExecTimeList:java.util.List[Long])
 
 @import com.linkedin.drelephant.analysis.Severity
@@ -35,7 +35,7 @@
 }
 
 @if(results != null && results.nonEmpty) {
-  @tags.panel(){ Job History Results: <a href="@jobDefId" style="font-size:14px; color:#083d8d">@jobDefId</a>} {
+  @tags.panel(){ Job History Results: <a href="@jobDefPair.getUrl" style="font-size:14px; color:#083d8d">@jobDefPair.getId</a>} {
     @if(graphType.equals("resources")) {
       <script src="@routes.Assets.at("js/jobresourcesmetricshistoryform.js")" type="text/javascript"></script>
       <script src="@routes.Assets.at("js/graphresourcesmetricsutility.js")" type="text/javascript"></script>