-
Notifications
You must be signed in to change notification settings - Fork 111
FALCON-1811 Status API does not honour start option #40
Conversation
| }); | ||
| }//Default : no sort | ||
|
|
||
| } |
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 "else" is missing here. Look like last block of sorting should be under "else".
Also since the code is redundant. You can simple change the condition on line 468 to if (orderBy.equals("starttime") || orderBy.isEmpty())
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.
looks valid making the change now..
|
+1. LGTM. |
| } | ||
| }); | ||
| } else if (orderBy.equals("starttime")){ | ||
| } else if (orderBy.equals("starttime") || orderBy.isEmpty()){ |
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.
This means we are ordering the results by startTime, by default. But, the current behavior is to order by instanceTime in descending order and this change breaks that.
|
Testing output : $ bin/falcon instance -type process -name process1 -start 2016-01-10T14:00Z -status Instances: Instance Cluster SourceCluster Status Start End Details Log2016-01-11T01:00Z local - SUSPENDED 2016-02-11T10:24Z - - http://falcon-server:11000/oozie?job=0000034-151229192519233-oozie-oozi-W |
| AbstractWorkflowEngine wfEngine = getWorkflowEngine(entityObject); | ||
| return getInstanceResultSubset(wfEngine.getStatus(entityObject, | ||
| startAndEndDate.first, startAndEndDate.second, lifeCycles, allAttempts), | ||
| InstancesResult instancesResult = getInstanceResultSubset(wfEngine.getStatus(entityObject, |
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.
Doesn't getInstanceResultSubset already apply numResults before returning? If there are i1-i20 instances and if startTime is i11 and numResults is 5, does this code return: i16 - i20 OR i11 - i15 ?
From the code changes, I think it will be i16-i20. But, what Pragya is asking for is i11-i15. Can you please confirm?
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.
Yes it used to return after applying numResults but I have move that code to this method from line number 310 to 184.Now it will give us i11-i15.
I have also pasted the same in the testing.
|
I believe this will require more changes:
|
|
+1. Sorry @PraveenAdlakha , I totally missed your update. Will merge right away. |
$ bin/falcon instance -type process -name process1 -start 2015-10-09T14:00Z -status -numResults 2 Consolidated Status: SUCCEEDED Instances: Instance Cluster SourceCluster Status Start End Details Log ----------------------------------------------------------------------------------------------- 2015-12-31T01:00Z local - SUSPENDED 2016-02-11T09:34Z - - http://blr-test-129.corp.inmobi.com:11000/oozie?job=0000028-151229192519233-oozie-oozi-W actions: mr-node null null failed-post-processing null null 2016-01-01T01:00Z local - SUSPENDED 2016-02-11T10:18Z - - http://blr-test-129.corp.inmobi.com:11000/oozie?job=0000030-151229192519233-oozie-oozi-W actions: mr-node null null failed-post-processing null null Additional Information: Response: default/STATUS Request Id: default/124451500qtp-822600088-12 - 2ca2d36d-6e03-4e14-9e97-1448aed90084 Note we will not get the same result when status is waiting as at that time start time is null. Author: Praveen Adlakha <adlakha.praveen@gmail.com> Reviewers: Pallavi Rao <pallavi.rao@inmobi.com> Closes apache#40 from PraveenAdlakha/1811 and squashes the following commits: d9925d0 [Praveen Adlakha] pallavi's comments addressed 7784468 [Praveen Adlakha] checkstyle voilation issue solved 4530233 [Praveen Adlakha] changes done as per pallavi's comment 3c3da61 [Praveen Adlakha] Deepak's comments addressed de08573 [Praveen Adlakha] FALCON-1811 Status API does not honour start option
$ bin/falcon instance -type process -name process1 -start 2015-10-09T14:00Z -status -numResults 2
Consolidated Status: SUCCEEDED
Instances:
Instance Cluster SourceCluster Status Start End Details Log
2015-12-31T01:00Z local - SUSPENDED 2016-02-11T09:34Z - - http://blr-test-129.corp.inmobi.com:11000/oozie?job=0000028-151229192519233-oozie-oozi-W
actions:
mr-node null null
failed-post-processing null null
2016-01-01T01:00Z local - SUSPENDED 2016-02-11T10:18Z - - http://blr-test-129.corp.inmobi.com:11000/oozie?job=0000030-151229192519233-oozie-oozi-W
actions:
mr-node null null
failed-post-processing null null
Additional Information:
Response: default/STATUS
Request Id: default/124451500@qtp-822600088-12 - 2ca2d36d-6e03-4e14-9e97-1448aed90084
Note we will not get the same result when status is waiting as at that time start time is null.