New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(CDAP-7420) Program lifecycle impersonation #6931
(CDAP-7420) Program lifecycle impersonation #6931
Conversation
} catch (Exception e) { | ||
return null; | ||
} | ||
} | ||
|
||
@Nullable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it actually ever return null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the logic, it's possible. And the caller also checks for it, hence I add the annotation to be clear.
YarnApplicationReport report = processController.getReport(); | ||
String host = report.getHost(); | ||
int port = report.getRpcPort(); | ||
if (host == null || host.equals("N/A") || port == -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should LOG#warn in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
LGTM; tested on cluster. Squashing commits. |
Refactor the ProgramRuntimeService and wrap the TwillController to minimize places that need to be aware of impersonation
f6908ee
to
b7ba023
Compare
Build green; merging |
Refactor the ProgramRuntimeService and wrap the TwillController to minimize places that need to be aware of impersonation