-
Notifications
You must be signed in to change notification settings - Fork 827
SOLR-18248 : Migration of Tasks V2 APIs to JAX-RS Constructs #4452
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
base: main
Are you sure you want to change the base?
Changes from 9 commits
acccb16
e426014
7b15791
02c14b9
177d2c6
95ace8d
7dc167a
1a32bb1
6b6f536
d746e21
df5c428
354d0f5
1cad09d
579925d
eb1cd97
4e10720
eb9b51b
adc7006
996b45b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package org.apache.solr.client.api.endpoint; | ||
|
|
||
| import io.swagger.v3.oas.annotations.Operation; | ||
| import jakarta.ws.rs.GET; | ||
| import jakarta.ws.rs.Path; | ||
| import jakarta.ws.rs.QueryParam; | ||
| import org.apache.solr.client.api.model.ListActiveTaskResponse; | ||
| import org.apache.solr.client.api.util.StoreApiParameters; | ||
|
|
||
| import static org.apache.solr.client.api.util.Constants.INDEX_PATH_PREFIX; | ||
|
|
||
| @Path(INDEX_PATH_PREFIX + "/tasks/listjalaz") | ||
| public interface ListActiveTasksApi { | ||
| @GET | ||
| @StoreApiParameters | ||
| @Operation( | ||
| summary = "Lists all the currently running tasks or status of any taskUUID being passed as queryParam", | ||
|
Contributor
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. So, I think we would revamp this api... if you look at the desired api, ListActiveTasks would just list all tasks. If you wanted a specfific task, then it would be a idfferent end point with a
Contributor
Author
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. yes, have updated this Api class for now, handling the taskUUID as a path param now, it was queryParam in the V1 as well as homegrown v2. |
||
| tags = {"tasks"}) | ||
| ListActiveTaskResponse listActiveTasks( | ||
| @QueryParam("taskUUID") String taskUUID) throws Exception; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package org.apache.solr.client.api.model; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import java.util.Map; | ||
|
|
||
| public class ListActiveTaskResponse extends SolrJerseyResponse { | ||
| @JsonProperty | ||
| public Map<String, String> taskList; | ||
|
Contributor
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. We are trying to move to more strongly typed responses so taht we don't have
Contributor
Author
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. Yes, have done so. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| package org.apache.solr.handler.admin.api; | ||
|
|
||
| import jakarta.inject.Inject; | ||
| import org.apache.solr.api.JerseyResource; | ||
| import org.apache.solr.client.api.endpoint.ListActiveTasksApi; | ||
| import org.apache.solr.client.api.model.ListActiveTaskResponse; | ||
| import org.apache.solr.core.CoreContainer; | ||
| import org.apache.solr.jersey.PermissionName; | ||
| import org.apache.solr.request.SolrQueryRequest; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.lang.invoke.MethodHandles; | ||
| import java.util.HashMap; | ||
| import java.util.Iterator; | ||
| import java.util.Map; | ||
|
|
||
| import static org.apache.solr.security.PermissionNameProvider.Name.READ_PERM; | ||
|
|
||
| public class ListActiveTasks extends JerseyResource implements ListActiveTasksApi { | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); | ||
|
|
||
| private final SolrQueryRequest solrQueryRequest; | ||
|
|
||
| @Inject | ||
| public ListActiveTasks( | ||
| SolrQueryRequest solrQueryRequest) { | ||
| this.solrQueryRequest = solrQueryRequest; | ||
| } | ||
|
|
||
|
|
||
|
|
||
| @Override | ||
| @PermissionName(READ_PERM) | ||
| public ListActiveTaskResponse listActiveTasks(String taskUUID) throws Exception { | ||
|
|
||
| final ListActiveTaskResponse response = instantiateJerseyResponse(ListActiveTaskResponse.class); | ||
| CoreContainer coreContainer = solrQueryRequest.getCoreContainer(); | ||
|
|
||
| if (coreContainer.isZooKeeperAware()) { | ||
| if (log.isDebugEnabled()) { | ||
| log.debug("solr cloud"); | ||
| } | ||
| handleSolrCloudMode(response, taskUUID); | ||
| } else { | ||
|
Contributor
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. not quite sure I see the differences in the two paths yet?
Contributor
Author
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. yea, i am also checking this up. |
||
| if (log.isDebugEnabled()) { | ||
| log.debug("standalone solr"); | ||
| } | ||
| handleStandAloneMode(response, taskUUID); | ||
| } | ||
|
|
||
| log.debug("something random"); | ||
|
|
||
| return response; | ||
|
|
||
| } | ||
|
|
||
| private void handleStandAloneMode(ListActiveTaskResponse response, String taskUUID) { | ||
|
Contributor
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. One thing we are trying to do is move the V1 API logic into the V2 code base, and have the V1 call the V2 version, this way, as these apis evolve, we know that v1 and v2 stay in sync till we get rid of V1. I believe that ideally we would want
Contributor
Author
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. understood this part, have made suitable changes. |
||
| Iterator<Map.Entry<String, String>> iterator = solrQueryRequest.getCore().getCancellableQueryTracker().getActiveQueriesGenerated(); | ||
|
|
||
| Map<String, String> taskList = new HashMap<>(); | ||
| while (iterator.hasNext()) { | ||
| Map.Entry<String, String> entry = iterator.next(); | ||
| taskList.put(entry.getKey(), entry.getValue()); | ||
| } | ||
|
|
||
| response.taskList = taskList; | ||
| } | ||
|
|
||
| private void handleSolrCloudMode(ListActiveTaskResponse response, String taskUUID) { | ||
| Iterator<Map.Entry<String, String>> iterator = solrQueryRequest.getCore().getCancellableQueryTracker().getActiveQueriesGenerated(); | ||
|
|
||
| Map<String, String> taskList = new HashMap<>(); | ||
| while (iterator.hasNext()) { | ||
| Map.Entry<String, String> entry = iterator.next(); | ||
| taskList.put(entry.getKey(), entry.getValue()); | ||
| } | ||
|
|
||
| response.taskList = taskList; | ||
| } | ||
| } | ||
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.
;-) Look slike you have some debugging?
Uh oh!
There was an error while loading. Please reload this page.
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.
yeah, started with firstly wiring this api to give any random response, & now using this to compare my V2 response with the original V2 response, 😄 thus, kept a different api endpoint for now to do response comparision.
will surely update this during self-review before raising for formal review 💯