-
Notifications
You must be signed in to change notification settings - Fork 117
Conversation
dev-tested |
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.
👍
@Produces({MediaType.TEXT_XML, MediaType.APPLICATION_JSON}) | ||
public ExtensionJobList getExtensionJobs( | ||
@PathParam("extension-name") String extensionName, | ||
@QueryParam("extension-name") String extensionName, |
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.
Why query param? Path param is REST compliant and is in-line with other APIs too.
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 API is used for two use case, one to get the jobs for an extension and other to get all jobs irrespective of registered extension.
In the second case, we were not passing anything in extension-name, means null.. which resulted in error from FalconClient that the given parameter is null. So I changed it to QueryParam.
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.
Understand the intent. Can you see if we can do something like what we do for entity listing. Entity type is part of the path, but, is optional.
@Produces({MediaType.TEXT_XML, MediaType.APPLICATION_JSON}) | ||
public ExtensionJobList getExtensionJobs( | ||
@PathParam("extension-name") String extensionName, | ||
@DefaultValue(ASCENDING_SORT_ORDER) @QueryParam("sortOrder") String sortOrder, | ||
@DefaultValue("") @QueryParam("doAs") String doAsUser) { | ||
checkIfExtensionServiceIsEnabled(); | ||
getExtensionIfExists(extensionName); | ||
if (extensionName != 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.
Use StringUtils.isNotBlank instead.
getExtensionIfExists(extensionName); | ||
if (StringUtils.isNotBlank(extensionName)) { | ||
getExtensionIfExists(extensionName); | ||
} | ||
try { | ||
return super.getExtensionJobs(extensionName, sortOrder, doAsUser); |
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.
Do you also want to modify getExtensionJobs to use StringUtils.isNotBlank? Because now, extensionName will not be 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.
smart catch. Ack.
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> Reviewers: @sandeepSamudrala,@pallavi-rao Closes #358 from PracheerAgarwal/FALCON-2274 and squashes the following commits: a5ac209 [Pracheer Agarwal] review comment changes 9abd11d [Pracheer Agarwal] review comment changes 53a49bd [Pracheer Agarwal] review comment changes 38f901e [Pracheer Agarwal] review comments changes 917d3d5 [Pracheer Agarwal] review comments changes e73e97d [Pracheer Agarwal] FALCON-2274 Job list in extension 0a9c51c [Pracheer Agarwal] FALCON-2274 Job list in extension ab1d136 [Pracheer Agarwal] FALCON-2274 Job list in extension ba60452 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon ed65aa0 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 9ff05df [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 9c2f0a5 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 9cd8c17 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 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 (cherry picked from commit b23b0a9) Signed-off-by: Pallavi Rao <pallavi.rao@inmobi.com>
Author: Pracheer Agarwal <pracheer.agarwal@inmobi.com> Author: Pracheer Agarwal <pracheeragarwal@gmail.com> Author: Pracheer Agarwal <pr@im2216-x0.corp.inmobi.com> Reviewers: @sandeepSamudrala,@pallavi-rao Closes apache#358 from PracheerAgarwal/FALCON-2274 and squashes the following commits: a5ac209 [Pracheer Agarwal] review comment changes 9abd11d [Pracheer Agarwal] review comment changes 53a49bd [Pracheer Agarwal] review comment changes 38f901e [Pracheer Agarwal] review comments changes 917d3d5 [Pracheer Agarwal] review comments changes e73e97d [Pracheer Agarwal] FALCON-2274 Job list in extension 0a9c51c [Pracheer Agarwal] FALCON-2274 Job list in extension ab1d136 [Pracheer Agarwal] FALCON-2274 Job list in extension ba60452 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon ed65aa0 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 9ff05df [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 9c2f0a5 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 9cd8c17 [Pracheer Agarwal] Merge branch 'master' of https://github.com/apache/falcon 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.