This repository has been archived by the owner on Apr 4, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 117
FALCON-2274 Job list in extension #358
Closed
Closed
Changes from 19 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
622cae4
FALCON-2225 extension owner added for trusted extensions
PracheerAgarwal-zz daa3ffc
FALCON-2225 extension owner added for trusted extensions
PracheerAgarwal-zz 46042fd
Merge branch 'master' of https://github.com/PracheerAgarwal/falcon
PracheerAgarwal-zz 7f572a1
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal-zz b20f044
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal-zz 066c8e2
Merge branch 'master' of https://github.com/apache/falcon
e3728d5
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal a93d71a
Merge branch 'master' of https://github.com/PracheerAgarwal/falcon
PracheerAgarwal fda3b28
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal-zz a932633
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal e39808d
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal-zz 778c579
Merge branch 'master' of https://github.com/PracheerAgarwal/falcon
PracheerAgarwal-zz 9cd8c17
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal-zz 9c2f0a5
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal-zz 9ff05df
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal-zz ed65aa0
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal-zz ba60452
Merge branch 'master' of https://github.com/apache/falcon
PracheerAgarwal-zz ab1d136
FALCON-2274 Job list in extension
PracheerAgarwal-zz 0a9c51c
FALCON-2274 Job list in extension
PracheerAgarwal-zz e73e97d
FALCON-2274 Job list in extension
PracheerAgarwal-zz 917d3d5
review comments changes
PracheerAgarwal-zz 38f901e
review comments changes
PracheerAgarwal-zz 53a49bd
review comment changes
PracheerAgarwal-zz 9abd11d
review comment changes
PracheerAgarwal-zz a5ac209
review comment changes
PracheerAgarwal-zz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,14 +94,16 @@ public class ExtensionManagerProxy extends AbstractExtensionManager { | |
|
||
//SUSPEND CHECKSTYLE CHECK ParameterNumberCheck | ||
@GET | ||
@Path("list/{extension-name}") | ||
@Path("list") | ||
@Produces({MediaType.TEXT_XML, MediaType.APPLICATION_JSON}) | ||
public ExtensionJobList getExtensionJobs( | ||
@PathParam("extension-name") String extensionName, | ||
@QueryParam("extension-name") String extensionName, | ||
@DefaultValue(ASCENDING_SORT_ORDER) @QueryParam("sortOrder") String sortOrder, | ||
@DefaultValue("") @QueryParam("doAs") String doAsUser) { | ||
checkIfExtensionServiceIsEnabled(); | ||
getExtensionIfExists(extensionName); | ||
if(extensionName != null) { | ||
getExtensionIfExists(extensionName); | ||
} | ||
try { | ||
return super.getExtensionJobs(extensionName, sortOrder, doAsUser); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. smart catch. Ack. |
||
} catch (Throwable e) { | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.