-
Notifications
You must be signed in to change notification settings - Fork 859
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
Fix missing Spark flow and job info. #198
Fix missing Spark flow and job info. #198
Conversation
@@ -147,4 +152,62 @@ public void testGetSchedulerInstanceNull() { | |||
assertEquals(null, scheduler); |
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.
assertNull
would be more obvious, I think, but I tried to limit the scope of the changes in this PR.
assertFalse(StringUtils.isEmpty(result.getJobDefUrl())); | ||
assertFalse(StringUtils.isEmpty(result.getFlowExecUrl())); | ||
assertFalse(StringUtils.isEmpty(result.getFlowDefUrl())); | ||
} |
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.
There's probably a Hamcrest matcher we could use here, but combining assertFalse
with StringUtils.isEmpty
seems to be good enough.
@@ -234,11 +234,12 @@ public static int getHeuristicScore(Severity severity, int tasks) { | |||
* | |||
* @param field the field to br truncated | |||
* @param limit the truncation limit | |||
* @param context additional context for logging purposes |
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 was wondering why truncateField
was taking an appId
, then I saw what was going on here. Hopefully this change helps.
properties = retrieveMapreduceProperties((MapReduceApplicationData) data); | ||
} | ||
|
||
Properties properties = data.getConf(); |
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.
After #162, getConf
should be sufficient for both MapReduceApplicationData
and SparkApplicationData
.
Closing this. Please check #162 for details. |
Fix missing Spark flow and job info (e.g. execution and definition URLs), which I broke after #162