[
https://issues.apache.org/jira/browse/OOZIE-2041?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15818382#comment-15818382
]
Attila Sasvari commented on OOZIE-2041:
---------------------------------------
Thanks [~abhishekbafna].
Some comments (most of them are minor):
In {{OozieCLI}}
- mention that parameters for the purge command are in days
In {{public String purgeCommand(String purgeOptions) throws
OozieClientException}}
- you could use {{Preconditions.checkNotNull}} from
{{com.google.common.base.Preconditions}} instead of the null check as a first
step.
- I believe using empty strings would better than using null Strings.
- Can you remove the commented out code in {{doPut}} {{setOozieMode}}
//Services.get().setSafeMode(safeMode);
- It would make sense to do some more validation here. For example we could
catch at the client side if invalid option is given (not a positive integer).
In {{schedulePurgeCommand}}
- I would start with
{code}
if (!ConfigurationService.getBoolean(PurgeService.PURGE_COMMAND_ENABLED) {
LOG.warn(purgeCmdDisabledMsg);
throw new XServletException(HttpServletResponse.SC_BAD_REQUEST,
ErrorCode.E0307, purgeCmdDisabledMsg);
}
...
String wfAgeStr = request.getParameter(RestConstants.PURGE_WF_AGE);
{code}
This would make cleaner i believe. If it is not an option please move
purgeCmdDisabledMsg declaration and initialization closer to where it is used.
- purgeCmdDisabledMsg could be a constant
In {{BaseAdminServlet}}
- update javadoc for {{doPut}} as it is not only used to "Change safemode
state." anymore.
I also plan to play a bit with the oozie cli command.
> Add an admin command to run the PurgeXCommand
> ---------------------------------------------
>
> Key: OOZIE-2041
> URL: https://issues.apache.org/jira/browse/OOZIE-2041
> Project: Oozie
> Issue Type: New Feature
> Components: core
> Affects Versions: trunk
> Reporter: Robert Kanter
> Assignee: Abhishek Bafna
> Fix For: 5.0.0
>
> Attachments: OOZIE-2041-00.patch, OOZIE-2041-01.patch,
> OOZIE-2041-02.patch, OOZIE-2041-03.patch, OOZIE-2041-04.patch,
> OOZIE-2041-05.patch
>
>
> Some users may find it useful to be able to run the PurgeXCommand on-demand.
> We can add a new admin command to run this. e.g.
> {noformat}
> oozie admin -purge
> {noformat}
> With no args, it can use the "older than" values from oozie-site, but we
> could have it take some arguments to override those for the command.
> Another thing to worry about is making sure that we don't run this if either
> it's already running (i.e. the user sent it twice) or if the scheduled
> PurgeService is already running it. On top of that, we may need extra
> consideration for HA setups where we currently only run it on the leader.
> We should probably also have a to not schedule the PurgeService in ooize-site
> for users who only want to run it manually.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
