[jira] [Commented] (YARN-7605) Implement doAs for Api Service REST API

["Jian He (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/YARN-7605?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16310519#comment-16310519 ] 

Jian He commented on YARN-7605:
-------------------------------

- we can't assume if result != 0, it is ApplicationNotFoundException ? and instead of "result.intValue() == 0", it can be "result == 0"
{code}
      if (result.intValue() == 0) {
        ServiceStatus serviceStatus = new ServiceStatus();
        serviceStatus.setDiagnostics("Successfully stopped service " +
            appName);
        return Response.status(Status.OK).entity(serviceStatus).build();
      } else {
        throw new ApplicationNotFoundException("Service " + appName +
            " is not found in YARN.");
      }
{code}
- what is this NullPointerException for ? we should fix the NPE instead ?
{code}
    } catch (NullPointerException | IOException e) {
      LOG.error("Fail to stop service:", e);
{code}
- similarly, I think it is inappropriate to assume the cause of all these exceptions is service not found
{code}
    } catch (InterruptedException | ApplicationNotFoundException |
        UndeclaredThrowableException e) {
      ServiceStatus serviceStatus = new ServiceStatus();
      serviceStatus.setDiagnostics("Service " + appName +
          " is not found in YARN.");
{code}
- it is fine to just return the result 
{code}
return Integer.valueOf(result);
{code}
- create a common method for these ?
{code}
      UserGroupInformation proxyUser = UserGroupInformation.getLoginUser();
      UserGroupInformation ugi = UserGroupInformation
          .createProxyUser(request.getRemoteUser(), proxyUser);
{code}
- Again, it is inappropriate to assume all exceptions are caused by "Permission denied" ? this will confuse the users. And why not just user LOG.warn("...", e), but use a seaprate log statement for the exception ?
{code}
    try {
      proxy.flexComponents(requestBuilder.build());
    } catch (YarnException | IOException e) {
      LOG.warn("Exception caught during flex operation.");
      LOG.warn(ExceptionUtils.getFullStackTrace(e));
      throw new YarnException("Permission denied to perform flex operation.");
    }
{code}
- why is it needed to add the InterruptedException to the method signature, and make callers catch this exception and then convert to YarnException ?
{code}
  private ApplicationId submitApp(Service app)
      throws IOException, YarnException, InterruptedException {
{code}
- unnecessary formatting change in createAMProxy, actionSave

> Implement doAs for Api Service REST API
> ---------------------------------------
>
>                 Key: YARN-7605
>                 URL: https://issues.apache.org/jira/browse/YARN-7605
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Eric Yang
>            Assignee: Eric Yang
>             Fix For: yarn-native-services
>
>         Attachments: YARN-7605.001.patch, YARN-7605.004.patch, YARN-7605.005.patch, YARN-7605.006.patch, YARN-7605.007.patch, YARN-7605.008.patch, YARN-7605.009.patch
>
>
> In YARN-7540, all client entry points for API service is centralized to use REST API instead of having direct file system and resource manager rpc calls.  This change helped to centralize yarn metadata to be owned by yarn user instead of crawling through every user's home directory to find metadata.  The next step is to make sure "doAs" calls work properly for API Service.  The metadata is stored by YARN user, but the actual workload still need to be performed as end users, hence API service must authenticate end user kerberos credential, and perform doAs call when requesting containers via ServiceClient.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org