-
Notifications
You must be signed in to change notification settings - Fork 117
FALCON-2247 include status of entities in getDetailExtensionJobs #345
Conversation
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.
Except for that minor comment. Looks good.
for (String feed : jobsBean.getFeeds()) { | ||
try { | ||
EntityUtil.getEntity(EntityType.FEED, feed); | ||
} catch (EntityNotRegisteredException e) { |
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.
only type is changed in these 2 segments. Extract a method for the same
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.
Test failures are unrelated to the changes.
try { | ||
EntityUtil.getEntity(type, entity); | ||
} catch (EntityNotRegisteredException e) { | ||
entityObject.put(entity, ENTITY_NOT_EXISTS_STATUS); |
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.
By JSON standards, the keys are usually constant strings. This should be { "name" : "abc, "status" : "EXISTS"}
This makes it easy to consume programmatically also.
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.
valid point. Have made the changes.
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.
👍
Author: Pracheer Agarwal <pracheer.agarwal@inmobi.com> Author: Pracheer Agarwal <pracheeragarwal@gmail.com> Author: Pracheer Agarwal <pr@im2216-x0.corp.inmobi.com> Author: sandeep <sandysmdl@gmail.com> Reviewers: @sandeepSamudrala, @pallavi-rao Closes apache#345 from PracheerAgarwal/FALCON-2247 and squashes the following commits: c57efa2 [sandeep] FALCON-2247 review commits changes 56492e7 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon into FALCON-2247 76215d7 [Pracheer Agarwal] changes for review comments c38c51b [Pracheer Agarwal] indentation changes 2ad4ce7 [Pracheer Agarwal] bug fixes 5d01f9c [Pracheer Agarwal] FALCON-2247 include status of entities in getDetailExtensionJobs 778c579 [Pracheer Agarwal] Merge branch 'master' of https://github.com/PracheerAgarwal/falcon e39808d [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon a932633 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon fda3b28 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon a93d71a [Pracheer Agarwal] Merge branch 'master' of https://github.com/PracheerAgarwal/falcon e3728d5 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 066c8e2 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon b20f044 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 7f572a1 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 46042fd [Pracheer Agarwal] Merge branch 'master' of https://github.com/PracheerAgarwal/falcon daa3ffc [Pracheer Agarwal] FALCON-2225 extension owner added for trusted extensions 622cae4 [Pracheer Agarwal] FALCON-2225 extension owner added for trusted extensions
No description provided.