-
Notifications
You must be signed in to change notification settings - Fork 1k
RANGER-5259: Add REST API to delete stale entries of Plugin info #683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
The changes look good, I think it would be good to add unit tests for the new API as well. thanks.
will keep the ticket open and add a follow up commit. |
@DELETE | ||
@Path("/plugins/info/{id}") | ||
@PreAuthorize("hasRole('ROLE_SYS_ADMIN')") | ||
public void deletePluginsInfo(@PathParam("id") Long id) { |
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.
Is it necessary to expose ithis method via REST, given id
is an internal field (i.e, generated by Ranger/DB)?
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 REST is designed keeping in the mind that option can be exposed in future on the RANGER UI to delete the selected row. Currently there is no option in the UI to delete the stale entries.
If this should not be exposed at all then we can remove the REST part of it and make it private.
if the concern is that such REST APIs should not be there then there are many such APIs which exists in ranger.
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.
ok. I suggest to introduce this API when we add UI for this.
@Path("/plugins/info") | ||
@Produces("application/json") | ||
@PreAuthorize("hasRole('ROLE_SYS_ADMIN')") | ||
public void deletePluginsInfo(@Context HttpServletRequest request) { |
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.
I suggest moving this method to AdminREST.
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 can be done.
deletePluginsInfo(rangerPluginInfo.getId()); | ||
LOG.debug("Deleted rangerPluginInfo:[{}]", rangerPluginInfo); | ||
} | ||
} |
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.
Add a info level log here:
LOG.info("Deleted {} pluginInfo records, for criteria={}", paginatedPluginsInfo.getList().size(), filter);
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.
Will do in the next patch.
What changes were proposed in this pull request?
We are adding two REST APIs in ranger-admin to delete stale plugin info entries:
Delete based on plugin info entry id : plugins/plugins/info/{id}
Example: if the plugin info entry id is 9 which need to be deleted then below REST shall work.
curl -ivk -u admin:Admin123 -H "Accept: application/json" -H "Content-Type: application/json" -X DELETE https://<ranger_host>:6182/service/plugins/plugins/info/9
Delete based on plugin info entry attributes : plugins/plugins/info
Case-1: if you want to delete all plugin info entries of a ranger service.
curl -ivk -u admin:Admin123 -H "Accept: application/json" -H "Content-Type: application/json" -X DELETE "https://<ranger_host>:6182/service/plugins/plugins/info?serviceName=${SERVICE_NAME}"
Case-2: if you want to delete all plugin info entries of a ranger service and host name.
curl -ivk -u admin:Admin123 -H "Accept: application/json" -H "Content-Type: application/json" -X DELETE "https://<ranger_host>:6182/service/plugins/plugins/info?serviceName=${SERVICE_NAME}&pluginHostName=${HOST_NAME}"
Case-3: if you want to delete all plugin info entries of a ranger service, host and application name/type.
curl -ivk -u admin:Admin123 -H "Accept: application/json" -H "Content-Type: application/json" -X DELETE "https://<ranger_host>:6182/service/plugins/plugins/info?serviceName=${SERVICE_NAME}&pluginHostName=${HOST_NAME}&pluginAppType=${APPLICATION_NAME}"
How was this patch tested?
Manually: via calling the above given REST point