You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/07/18 10:35:53 UTC

[GitHub] [solr] ijioio commented on a diff in pull request #931: SOLR-16282: Update core admin handler to use pluggable custom actions

ijioio commented on code in PR #931:
URL: https://github.com/apache/solr/pull/931#discussion_r923208054


##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java:
##########
@@ -399,19 +424,16 @@ public void shutdown() {
     if (parallelExecutor != null) ExecutorUtil.shutdownAndAwaitTermination(parallelExecutor);
   }
 
-  private static final Map<String, CoreAdminOperation> opMap = new HashMap<>();
+  private static final Map<String, CoreAdminOp> opMap = new HashMap<>();
 
-  static class CallInfo {
-    final CoreAdminHandler handler;
-    final SolrQueryRequest req;
-    final SolrQueryResponse rsp;
-    final CoreAdminOperation op;
+  public static class CallInfo {
+    public final CoreAdminHandler handler;
+    public final SolrQueryRequest req;
+    public final SolrQueryResponse rsp;
+    public final CoreAdminOp op;
 
     CallInfo(
-        CoreAdminHandler handler,
-        SolrQueryRequest req,
-        SolrQueryResponse rsp,
-        CoreAdminOperation op) {
+        CoreAdminHandler handler, SolrQueryRequest req, SolrQueryResponse rsp, CoreAdminOp op) {

Review Comment:
   > Left couple of minor comments inline.
   
   Thank you, it is great to hear other people opinion! 
   
   > And next steps then could be documentation updates and/or test coverage e.g. configuring-solr-xml.adoc and solr-50-all.xml maybe?
   
   Yes, that preferably should be done., I doesn't create tests for Solr before, for some reason when I try to run tests task it takes a huge amount of time and I have never succeeded to wait until it is ends %) If you have some nice example of tests to base it on please advice. I will try to create some if we decided to proceed with the MR.
   
   > making the class and its elements public is backwards compatible I'd say
   
   Yes, I was trying to keep original code as much compatible as possible not to apply too much changes...
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org