tez-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hitesh Shah (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (TEZ-2018) Job Tracking URL should point to the Tez UI
Date Tue, 03 Feb 2015 19:24:37 GMT

    [ https://issues.apache.org/jira/browse/TEZ-2018?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14303803#comment-14303803

Hitesh Shah commented on TEZ-2018:

  - why is the jetty version downgraded?
  - why does a user need to configure both "TEZ_HISTORY_URL_BASE" and "TEZ_AM_TEZ_UI_HISTORY_URL_TEMPLATE"
? Is TEZ_AM_TEZ_UI_HISTORY_URL_TEMPLATE meant for internal use only? 
     - Documentation needs to be more clear as it references internal class fields instead
of config properties
  - why is this TEZ_AM_WEBSERVICE_ENABLE needed?

  - Is hadoop-yarn-server-web-proxy a public or a private package? Does yarn guarantee compatibility?

String historyUrl;
524	    if (webUI != null) {
525	      historyUrl = webUI.getHistoryUrl();
526	    } else {
527	      historyUrl = "";
528	    }
  - could be re-written to "()? x : y; " similar to trackingUrl handling earlier in the code.

origin = url.getProtocol() + "://" + url.getAuthority();
  - Will there be issues when handling https? If yes, please file a follow-up jira to support

 - Please wrap all "LOG.debug" within a if (LOG.isDebugEnabled()) 

      sendErrorResponse(HttpServletResponse.SC_UNAUTHORIZED, "Access denied", null);
  - Might be good to have the message contain the user id.

pw.write("<p>The TEZ UI is hosted at <a href=\"" + historyUrl + "\">"
314	          + historyUrl + "</href>");
  - does this message need to be modified to mention that the page will automatically redirect
or to click the link if it does not redirect? 

  if (historyUrl == null || historyUrl.isEmpty()) {
  // set the tracking url only if history url is set, as the index page will redirect to
  // history url
  - Better handling for this case is needed. The trackingUrl for the AM should still be set
but the html which is rendered should inform the user that a history url was not configured
and also tell the user what config property should be set. 

// Explicitly disabling SSL for map reduce task as we can't allow AM users
  - there is a map reduce related comment. Copy-paste error? 

    boolean atsEnabled = config.getBoolean(YarnConfiguration.TIMELINE_SERVICE_ENABLED, false);
  - why are we relying on a yarn config? If tez is configured to use the timeline logging
service, that should suffice as a check.

historyUrl = historyUrlTemplate
157	          .replaceAll(APPLICATION_ID_PLACEHOLDER, this.context.getApplicationID().toString())
158	          .replaceAll(HISTORY_URL_BASE, historyUrlBase);
  - Can HISTORY_URL_BASE itself be a template?    

 - Do we really need both vertexProgresses() and vertexProgress() - isn't the former the superset
of the latter? 

 conf.set(TezConfiguration.TEZ_AM_TEZ_UI_HISTORY_URL_TEMPLATE ,
  - is there a reason why this needs to be set in the test? Shouldn't the code fall back to
using the default if  TEZ_AM_TEZ_UI_HISTORY_URL_TEMPLATE is not set? 

Patch also introduces new findbugs warnings. 


> Job Tracking URL should point to the Tez UI
> -------------------------------------------
>                 Key: TEZ-2018
>                 URL: https://issues.apache.org/jira/browse/TEZ-2018
>             Project: Apache Tez
>          Issue Type: New Feature
>            Reporter: Rohini Palaniswamy
>            Assignee: Prakash Ramachandran
>            Priority: Critical
>         Attachments: TEZ-2018.1.patch, TEZ-2018.wip.1.patch
>     We need to have the Job Tracking URL take users to the Tez UI if yarn.timeline-service.enabled=true

This message was sent by Atlassian JIRA

View raw message