[ https://issues.apache.org/jira/browse/YARN-2233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14048865#comment-14048865 ]
Zhijie Shen commented on YARN-2233: ----------------------------------- Thanks Varun for the patch. In general, the patch looks good, and I like the detailed test cases:-) Here're some point I'd like to you help to further clarify: 1. bq. It should be noted that when cancelling a token, the token to be cancelled is specified by setting a header. Any reason for specifying the token in head? If there's something non-intuitive, maybe we should have some in-code comments for other developers? 2. RPC get delegation token API doesn't have these fields, but it seems to be nice have. We may want to file a Jira. {code} + long currentExpiration = ident.getIssueDate() + tokenRenewInterval; + long maxValidity = ident.getMaxDate(); {code} 3. Is it possible to reuse KerberosTestUtils in hadoop-auth? 4. Is this supposed to test invalid request body? It doesn't look like the invalid body construction in the later tests. {code} + response = + resource().path("ws").path("v1").path("cluster") + .path("delegation-token").accept(contentType) + .entity(dtoken, mediaType).post(ClientResponse.class); + assertEquals(Status.BAD_REQUEST, response.getClientResponseStatus()); {code} Some minor issues: 1. No need of "== ture". {code} + if (usePrincipal == true) { {code} Similarly, {code} + if (KerberosAuthenticationHandler.TYPE.equals(authType) == false) { {code} 2. If I remember it correctly, callerUGI.doAs will throw UndeclaredThrowableException, which wraps the real raised exception. However, UndeclaredThrowableException is an RE, this code cannot capture it. {code} + try { + resp = + callerUGI + .doAs(new PrivilegedExceptionAction<GetDelegationTokenResponse>() { + @Override + public GetDelegationTokenResponse run() throws IOException, + YarnException { + GetDelegationTokenRequest createReq = + GetDelegationTokenRequest.newInstance(renewer); + return rm.getClientRMService().getDelegationToken(createReq); + } + }); + } catch (Exception e) { + LOG.info("Create delegation token request failed", e); + throw e; + } {code} 3. Cannot return respToken simply? The framework should generate "OK" status automatically, right? {code} + return Response.status(Status.OK).entity(respToken).build(); {code} 4. You can call tk.decodeIdentifier directly. {code} + RMDelegationTokenIdentifier ident = new RMDelegationTokenIdentifier(); + ByteArrayInputStream buf = new ByteArrayInputStream(tk.getIdentifier()); + DataInputStream in = new DataInputStream(buf); + ident.readFields(in); {code} > Implement web services to create, renew and cancel delegation tokens > -------------------------------------------------------------------- > > Key: YARN-2233 > URL: https://issues.apache.org/jira/browse/YARN-2233 > Project: Hadoop YARN > Issue Type: Sub-task > Reporter: Varun Vasudev > Assignee: Varun Vasudev > Attachments: apache-yarn-2233.0.patch > > > Implement functionality to create, renew and cancel delegation tokens. -- This message was sent by Atlassian JIRA (v6.2#6252)