You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "sayandigital (via GitHub)" <gi...@apache.org> on 2023/03/11 06:04:41 UTC

[GitHub] [solr] sayandigital opened a new pull request, #1449: SOLR-16490 Create a v2 equivalent for RESTORECORE

sayandigital opened a new pull request, #1449:
URL: https://github.com/apache/solr/pull/1449

   https://issues.apache.org/jira/browse/SOLR-16490
   
   <!--
   _(If you are a project committer then you may remove some/all of the following template.)_
   
   Before creating a pull request, please file an issue in the ASF Jira system for Solr:
   
   * https://issues.apache.org/jira/projects/SOLR
   
   For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA. 
   To create a Jira issue, you will need to create an account there first.
   
   The title of the PR should reference the Jira issue number in the form:
   
   * SOLR-16490: Create a v2 equivalent for /admin/cores RESTORECORE
   
   SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
   
   Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
   
   
   # Description
   
   Create a v2 equivalent for /admin/cores for RESTORECORE 
   
   # Solution
   
   Adding a new class called RestoreCoreAPI and RestoreCorePayload as per the V2 API documentation provided in the document( Solr v2 API proposed changes https://docs.google.com/spreadsheets/d/1HAoBBFPpSiT8mJmgNZKkZAPwfCfPvlc08m5jz3fQBpA/edit#gid=1549066254)
   
   <google-sheets-html-origin><style type="text/css"><!--td {border: 1px solid #cccccc;}br {mso-data-placement:same-cell;}--></style>
   
   Restore Core | GET /solr/admin/cores?action=RESTORECORE&core=coreName | POST /api/cores/coreName {"restore": {...}} | POST /api/cores/coreName/restore | CoreAdminHandler | RestoreCoreAPI | TRUE | FALSE | FALSE | SOLR-16490 |   |   |   |   |   |   |   |   |   |   |   |   |   |   |   |   |   |   |   |  
   -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | --
   
   
   
   
   # Tests
   
   Added test cases in solr/core/src/test/org/apache/solr/handler/admin/api/V2CoreAPIMappingTest.java
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [ Y] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [ Y] I have created a Jira issue and added the issue ID to my pull request title.
   - [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [ Y] I have developed this patch against the `main` branch.
   - [ Y] I have run `./gradlew check`.
   - [ Y] I have added tests for my changes.
   - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


-- 
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

[GitHub] [solr] gerlowskija merged pull request #1449: SOLR-16490 Create a v2 equivalent for RESTORECORE

Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija merged PR #1449:
URL: https://github.com/apache/solr/pull/1449


-- 
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

[GitHub] [solr] gerlowskija commented on a diff in pull request #1449: SOLR-16490 Create a v2 equivalent for RESTORECORE

Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on code in PR #1449:
URL: https://github.com/apache/solr/pull/1449#discussion_r1156337270


##########
solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCoreAPI.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.admin.api;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CoreAdminParams.ACTION;
+import static org.apache.solr.common.params.CoreAdminParams.CORE;
+import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.RESTORECORE;
+import static org.apache.solr.handler.ClusterAPI.wrapParams;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.RestoreCorePayload;
+import org.apache.solr.handler.admin.CoreAdminHandler;
+
+/**
+ * Internal V2 API triggering restore a core.
+ *
+ * <p>Only valid in SolrCloud mode. This API (POST /api/cores/coreName/restore {'restore': {}}) is
+ * analogous to the v1 GET /solr/admin/cores?action=RESTORECORE command.
+ *
+ * @see RestoreCorePayload
+ */
+@EndPoint(
+    path = {"/cores/{core}"},

Review Comment:
   [-1] IMO the endpoint specifics (particularly the path/url) should look a little different here.
   
   It might've gotten buried in all the text on SOLR-16490, but we've been building up [a spreadsheet](https://docs.google.com/spreadsheets/d/1HAoBBFPpSiT8mJmgNZKkZAPwfCfPvlc08m5jz3fQBpA) of Solr's APIs and what their v2 equivalents will ideally look like.  RESTORECORE is a bit hard to find there, but on the "'Command' APIs" Tab there's an entry for it.  The path there is listed as `/cores/{core}/restore`- could we use that value here?



##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/RestoreCorePayload.java:
##########
@@ -0,0 +1,22 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.client.solrj.request.beans;
+
+import org.apache.solr.common.util.ReflectMapWriter;
+
+public class RestoreCorePayload implements ReflectMapWriter {}

Review Comment:
   [0] RestoreCore operations supports a bunch of params that we'll need to include in the payload/request-body here.  For a sense of what all 'restore' supports, see the `req.getParams()` calls in `RestoreCoreOp`



##########
solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCoreAPI.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.admin.api;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.params.CoreAdminParams.ACTION;
+import static org.apache.solr.common.params.CoreAdminParams.CORE;
+import static org.apache.solr.common.params.CoreAdminParams.CoreAdminAction.RESTORECORE;
+import static org.apache.solr.handler.ClusterAPI.wrapParams;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import org.apache.solr.api.Command;
+import org.apache.solr.api.EndPoint;
+import org.apache.solr.api.PayloadObj;
+import org.apache.solr.client.solrj.request.beans.RestoreCorePayload;
+import org.apache.solr.handler.admin.CoreAdminHandler;
+
+/**
+ * Internal V2 API triggering restore a core.
+ *
+ * <p>Only valid in SolrCloud mode. This API (POST /api/cores/coreName/restore {'restore': {}}) is
+ * analogous to the v1 GET /solr/admin/cores?action=RESTORECORE command.
+ *
+ * @see RestoreCorePayload
+ */
+@EndPoint(
+    path = {"/cores/{core}"},
+    method = POST,
+    permission = CORE_EDIT_PERM)
+public class RestoreCoreAPI {
+
+  public static final String V2_CORE_RESTORE_CMD = "restore";
+
+  private final CoreAdminHandler coreAdminHandler;
+
+  public RestoreCoreAPI(CoreAdminHandler coreAdminHandler) {
+    this.coreAdminHandler = coreAdminHandler;
+  }
+
+  @Command(name = V2_CORE_RESTORE_CMD)

Review Comment:
   [-1] Similar my comment above about the API path, I think the overall shape of this code looks great.  Just need to tweak a few specifics so that the v2 RESTORECORE API lines up better with the direction we're trying to move our v2 APIs in.
   
   Particularly, we're trying to move away from the v2 "command-based" APIs (i.e. APIs with a request body whose only top-level key is the name of some command or other).  Instead we're generally moving the command itself into the path (see my comment above about changing the path to `/cores/coreName/restore`) and then moving most of the actual restore parameters to the top level of the "Payload"/request-body.
   
   This is definitely do-able using the Solr-custom annotation framework you've used in this PR (see RenameCollectionAPI for an example).  But you might find it more natural to do if you switch this API over to use the JAX-RS framework we've been moving towards.  If you want to try that out there's a step-by-step writeup on this [here](https://issues.apache.org/jira/browse/SOLR-15737?focusedCommentId=17617923&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17617923) and a handful of examples that you can crib from ([example1](https://github.com/apache/solr/pull/1452), [example2](https://github.com/apache/solr/pull/1441))



-- 
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

[GitHub] [solr] sayandigital commented on pull request #1449: SOLR-16490 Create a v2 equivalent for RESTORECORE

Posted by "sayandigital (via GitHub)" <gi...@apache.org>.
sayandigital commented on PR #1449:
URL: https://github.com/apache/solr/pull/1449#issuecomment-1464858331

   @gerlowskija 


-- 
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

[GitHub] [solr] gerlowskija commented on pull request #1449: SOLR-16490 Create a v2 equivalent for RESTORECORE

Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #1449:
URL: https://github.com/apache/solr/pull/1449#issuecomment-1494538121

   Hi @sayandigital - sorry for the delay, this got buried in my Inbox for awhile.  But thanks for picking this up; will aim to review shortly!


-- 
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

[GitHub] [solr] gerlowskija commented on pull request #1449: SOLR-16490 Create a v2 equivalent for RESTORECORE

Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #1449:
URL: https://github.com/apache/solr/pull/1449#issuecomment-1664231709

   Alright - I've made some changes on top of the base you provided @sayandigital.  Both to address some of the review comments, and to use the more-preferred JAX-RS framework that Solr supports now.  Tests and 'check' pass; gonna merge this afternoon and then let things bake for a few days before backporting.


-- 
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

[GitHub] [solr] sayandigital commented on pull request #1449: SOLR-16490 Create a v2 equivalent for RESTORECORE

Posted by "sayandigital (via GitHub)" <gi...@apache.org>.
sayandigital commented on PR #1449:
URL: https://github.com/apache/solr/pull/1449#issuecomment-1494558122

   > Hi @sayandigital - sorry for the delay, this got buried in my Inbox for awhile. But thanks for picking this up; will aim to review shortly!
   
   Thank you @gerlowskija 


-- 
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

[GitHub] [solr] gerlowskija commented on pull request #1449: SOLR-16490 Create a v2 equivalent for RESTORECORE

Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #1449:
URL: https://github.com/apache/solr/pull/1449#issuecomment-1548414128

   Hi @sayandigital - thanks again for your work on this.  Let me know if I can help out with any questions or issues that crop up when you get a chance to circle back to this!


-- 
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

[GitHub] [solr] sayandigital commented on pull request #1449: SOLR-16490 Create a v2 equivalent for RESTORECORE

Posted by "sayandigital (via GitHub)" <gi...@apache.org>.
sayandigital commented on PR #1449:
URL: https://github.com/apache/solr/pull/1449#issuecomment-1689254035

   Thanks @gerlowskija to help take this to completion


-- 
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