[
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: [email protected]
For additional commands, e-mail: [email protected]
