FALCON-1566 Add test for SLA monitoring API #44
Conversation
| if (tagDiff != 0) { | ||
| return tagDiff; | ||
| } | ||
| int clusterDiff = o1.getCluster().compareTo(o2.getCluster()); |
There was a problem hiding this comment.
I think you can remove lines 86-102 by simply "return o1.compareTo(o2)".
Also, I 'm not sure why tags are not compared in SchedulableEntityInstance.compareTo() method. If tags should be compared in that SchedulableEntityInstance class, you can remove lines 82-85 as well. What do you think ?
There was a problem hiding this comment.
I intended to use compareTo but since it tags comparison was missing, overrided it by my method. Anyways I can use compareTo with tags handled explicitly. Will make the changes.
There was a problem hiding this comment.
Not sure why tags are not compared in SchedulableEntityInstance.compareTo(). If it should be there, It's better to add tag comparison to SchedulableEntityInstance.compareTo(). Otherwise, people might end up doing tag comparison explicitly at multiple places when they feel the need. Not a good way.
There was a problem hiding this comment.
I agree with your point. Created #45 for the same.
|
👍 looks good! |
Author: Pragya <mittal.pragya23@gmail.com> Reviewers: Paul Isaychuk <pisaychuk@apache.org>, Deepak Kumar Barr <deepak.barr@gmail.com> Closes apache#44 from pragya-mittal/feed-sla and squashes the following commits: c6c6f65 [Pragya] Review comments addressed 251d404 [Pragya] Resolved merge conflicts f5b2888 [Pragya] FALCON-1566 Add test for SLA monitoring API f037385 [Pragya] Merge branch 'master' of https://github.com/apache/falcon 4c19ec0 [Pragya] Merge branch 'master' of https://github.com/apache/falcon 3b7fd63 [Pragya] FALCON-1829 Add regression for submit and schedule process on native scheduler (time based)
3 participants
No description provided.