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

[GitHub] [solr] Jdurham2843 opened a new pull request, #1471: Solr 15737 snapshot collection v2

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

   https://issues.apache.org/jira/browse/SOLR-XXXXX
   
   <!--
   _(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-####: <short description of problem or changes>
   
   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
   
   This PR contains the V2 API implementations of CREATE, LIST, and DELETE collection snapshot resources.
   
   # Solution
   
   I created Jersey implementations of the existing CREATE, LIST, and DELETE collection snapshot resources. I then refactored the existing V1 endpoints to call the new V2 resources.
   
   # Tests
   
   TODO: I plan to write tests that excercise the V2 resources.
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [x] 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.
   - [x] I have created a Jira issue and added the issue ID to my pull request title.
   - [x] 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)
   - [x] I have developed this patch against the `main` branch.
   - [x] I have run `./gradlew check`.
   - [ ] 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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */

Review Comment:
   Lazy copy-paste on my end, will address
   
   (Did you leave off the parentheses on your last line above on purpose?) :) 



-- 
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 #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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

   > Is there another way to pull in changes, without needing to force push?
   
   Yep, definitely!  IMO this stuff can be tricky to explain (or read about) through text alone.  Everyone's git settings and repo tends to be just different enough that sharing workflow-commands alone can cause as much confusion as it clears up.  Anyway, if this explanation doesn't help I'm happy to do a screenshare to discuss as well. (My email address should be on my GH profile)
   
   But here goes!
   
   ----
   
   Let's say I have a feature branch (`my-feature`) on a Github fork repo (`myfork`), and I want to pull in some changes from the `main` branch in the Apache repo (`origin`).
   
   * The first step is to update your local copy of `main` so that it matches the latest stuff upstream:
      * `git checkout main`
      * `git pull origin main
   * Now that your local copy of `main` is up to date, you can merge those changes into your feature branch
      * `git checkout my-feature`
      * `git merge main`
      * (Optionally fix any merge conflicts that might've arisen and `git commit` when you're happy)
   * Your local `my-feature` branch now has the changes from `main`, we just need to publish your updated branch upstream:
      * `git push myfork my-feature:my-feature`
      
   Hope that helps!


-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")

Review Comment:
   I think that makes a lot of sense, when the intent of the API is to create or update a resource. In that case it feels like things that affect "how" the work is done should be kept separate from the thing being acted on. Although, you could have a POST that falls more into the category of doing work within the "system" instead of just on a resource. Things that tweak that work could still come in via query-parameters, but a request-body may be more convenient. I guess in that scenario your "resource" is the "system"?  In any case, I think either approach could be fine, as long as its consistent.



-- 
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 #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")

Review Comment:
   OK, sounds good.  I agree with keeping it in the request body for now.
   
   To think aloud a little bit here...
   
   I've been mulling over for awhile now the idea of changing 'async' to be a query param instead of a property in the request-body.  I think the intention in RESTful APIs is that the request-body of a PUT/POST is some representation of the "resource" being created.  'async' doesn't quite fit that conceptually - it's focused less on "what" resource is created and more on "how" the resource is created.  But I'm still undecided personally, and it'll need changed in a few places even if we do decide to go that route.  So definitely something to skip in terms of this PR.



-- 
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 #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1764,96 +1767,55 @@ public Map<String, Object> execute(
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
-          String collectionName =
-              followAliases
-                  ? h.coreContainer
-                      .getZkController()
-                      .getZkStateReader()
-                      .getAliases()
-                      .resolveSimpleAlias(extCollectionName)
-                  : extCollectionName;
-          String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          if (SolrSnapshotManager.snapshotExists(client, collectionName, commitName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Snapshot with name '"
-                    + commitName
-                    + "' already exists for collection '"
-                    + collectionName
-                    + "', no action taken.");
-          }
+          final CreateCollectionSnapshotAPI createCollectionSnapshotAPI =
+              new CreateCollectionSnapshotAPI(h.coreContainer, req, rsp);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final CreateCollectionSnapshotAPI.CreateSnapshotResponse createSnapshotResponse =
+              createCollectionSnapshotAPI.createSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, createSnapshotResponse);
+
+          return null;
         }),
     DELETESNAPSHOT_OP(
         DELETESNAPSHOT,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final DeleteCollectionSnapshotAPI deleteCollectionSnapshotAPI =
+              new DeleteCollectionSnapshotAPI(h.coreContainer, req, rsp);
+
+          final DeleteCollectionSnapshotAPI.DeleteSnapshotResponse deleteSnapshotResponse =
+              deleteCollectionSnapshotAPI.deleteSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, deleteSnapshotResponse);
+          return null;
         }),
     LISTSNAPSHOTS_OP(
         LISTSNAPSHOTS,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final ListCollectionSnapshotsAPI listCollectionSnapshotsAPI =
+              new ListCollectionSnapshotsAPI(h.coreContainer, req, rsp);
+
+          final ListCollectionSnapshotsAPI.ListSnapshotsResponse response =
+              listCollectionSnapshotsAPI.listSnapshots(req.getParams().get(COLLECTION_PROP));
 
-          NamedList<Object> snapshots = new NamedList<Object>();
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          Collection<CollectionSnapshotMetaData> m =
-              SolrSnapshotManager.listSnapshots(client, collectionName);
-          for (CollectionSnapshotMetaData meta : m) {
+          NamedList<Object> snapshots = new NamedList<>();
+          for (CollectionSnapshotMetaData meta : response.snapshots.values()) {

Review Comment:
   Ah, ok.  Actually this makes complete sense.  The "squash" pattern only knows how to serialize particular types and interfaces, and I guess CollectionSnapshotMetaData isn't one of those.  Forget I said anything, as long as the output looks correct 👍 



-- 
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 #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1764,96 +1767,55 @@ public Map<String, Object> execute(
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
-          String collectionName =
-              followAliases
-                  ? h.coreContainer
-                      .getZkController()
-                      .getZkStateReader()
-                      .getAliases()
-                      .resolveSimpleAlias(extCollectionName)
-                  : extCollectionName;
-          String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          if (SolrSnapshotManager.snapshotExists(client, collectionName, commitName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Snapshot with name '"
-                    + commitName
-                    + "' already exists for collection '"
-                    + collectionName
-                    + "', no action taken.");
-          }
+          final CreateCollectionSnapshotAPI createCollectionSnapshotAPI =
+              new CreateCollectionSnapshotAPI(h.coreContainer, req, rsp);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final CreateCollectionSnapshotAPI.CreateSnapshotResponse createSnapshotResponse =
+              createCollectionSnapshotAPI.createSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, createSnapshotResponse);
+
+          return null;
         }),
     DELETESNAPSHOT_OP(
         DELETESNAPSHOT,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final DeleteCollectionSnapshotAPI deleteCollectionSnapshotAPI =
+              new DeleteCollectionSnapshotAPI(h.coreContainer, req, rsp);
+
+          final DeleteCollectionSnapshotAPI.DeleteSnapshotResponse deleteSnapshotResponse =
+              deleteCollectionSnapshotAPI.deleteSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, deleteSnapshotResponse);
+          return null;
         }),
     LISTSNAPSHOTS_OP(
         LISTSNAPSHOTS,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final ListCollectionSnapshotsAPI listCollectionSnapshotsAPI =
+              new ListCollectionSnapshotsAPI(h.coreContainer, req, rsp);
+
+          final ListCollectionSnapshotsAPI.ListSnapshotsResponse response =
+              listCollectionSnapshotsAPI.listSnapshots(req.getParams().get(COLLECTION_PROP));
 
-          NamedList<Object> snapshots = new NamedList<Object>();
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          Collection<CollectionSnapshotMetaData> m =
-              SolrSnapshotManager.listSnapshots(client, collectionName);
-          for (CollectionSnapshotMetaData meta : m) {
+          NamedList<Object> snapshots = new NamedList<>();
+          for (CollectionSnapshotMetaData meta : response.snapshots.values()) {

Review Comment:
   [Q] This code is simple enough, but unless I'm missing something it seems like it could be replaced by the `V2ApiUtils.squashIntoSolrResponse` pattern we use on other APIs.  Any particular reason you didn't go that route here?



##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */

Review Comment:
   [0] Minor typo, no closing paren around the v1 API endpoint.
   
   (Also appears in the LIST and DELETE files



##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")
+          boolean followAliases,
+      @QueryParam("asyncId") String asyncId)
+      throws Exception {
+
+    final CreateSnapshotResponse response = instantiateJerseyResponse(CreateSnapshotResponse.class);
+    final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+    recordCollectionForLogAndTracing(collName, solrQueryRequest);
+
+    final String collectionName =
+        followAliases
+            ? coreContainer
+                .getZkController()
+                .getZkStateReader()
+                .getAliases()
+                .resolveSimpleAlias(collName)
+            : collName;
+
+    final ClusterState clusterState = coreContainer.getZkController().getClusterState();
+    if (!clusterState.hasCollection(collectionName)) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Collection '" + collectionName + "' does not exist, no action taken.");
+    }

Review Comment:
   [0] If you're up for it, this pattern (lines 89-103) happens enough in our "collection admin" APIs that there'd probably be some value in creating a method for it in AdminAPIBase.



##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")

Review Comment:
   [Q] Did you have a particular reason you made these values query-params, as opposed to values in a JSON request body or something?
   
   I'm not against having these be query-params, but it does conflict a bit with the handful of other POST/PUT APIs that create some resource/entity in Solr. There's definitely a lot I don't know about API design, was curious if you had something particular in mind...



##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")
+          boolean followAliases,
+      @QueryParam("asyncId") String asyncId)
+      throws Exception {
+
+    final CreateSnapshotResponse response = instantiateJerseyResponse(CreateSnapshotResponse.class);
+    final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+    recordCollectionForLogAndTracing(collName, solrQueryRequest);
+
+    final String collectionName =
+        followAliases
+            ? coreContainer
+                .getZkController()
+                .getZkStateReader()
+                .getAliases()
+                .resolveSimpleAlias(collName)
+            : collName;
+
+    final ClusterState clusterState = coreContainer.getZkController().getClusterState();
+    if (!clusterState.hasCollection(collectionName)) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Collection '" + collectionName + "' does not exist, no action taken.");
+    }
+
+    final SolrZkClient client = coreContainer.getZkController().getZkClient();
+    if (SolrSnapshotManager.snapshotExists(client, collectionName, snapshotName)) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Snapshot with name '"
+              + snapshotName
+              + "' already exists for collection '"
+              + collectionName
+              + "', no action taken.");
+    }
+
+    final ZkNodeProps remoteMessage =
+        createRemoteMessage(collName, followAliases, snapshotName, asyncId);
+    final SolrResponse remoteResponse =
+        CollectionsHandler.submitCollectionApiCommand(
+            coreContainer,
+            coreContainer.getDistributedCollectionCommandRunner(),
+            remoteMessage,
+            CollectionParams.CollectionAction.CREATESNAPSHOT,
+            DEFAULT_COLLECTION_OP_TIMEOUT);
+
+    if (remoteResponse.getException() != null) {
+      throw remoteResponse.getException();
+    }
+
+    response.collection = collName;
+    response.followAliases = followAliases;
+    response.snapshotName = snapshotName;
+    response.requestId = asyncId;
+
+    return response;
+  }
+
+  /**
+   * The Response for {@link CreateCollectionSnapshotAPI}'s {@link #createSnapshot(String, String,
+   * boolean, String)}
+   */
+  public static class CreateSnapshotResponse extends SolrJerseyResponse {
+    @Schema(description = "The name of the collection.")
+    @JsonProperty(COLLECTION_PROP)
+    String collection;
+
+    @Schema(description = "The name of the snapshot to be created.")
+    @JsonProperty("snapshot")
+    String snapshotName;
+
+    @Schema(description = "A flag that treats the collName parameter as a collection alias.")
+    @JsonProperty("followAliases")
+    boolean followAliases;
+
+    @JsonProperty("requestId")
+    String requestId;
+  }
+
+  private ZkNodeProps createRemoteMessage(

Review Comment:
   [0] There's definitely a tradeoff in raising the visibility of this method, but I've found it useful to make these "createRemoteMessage" implementations `public static` so that it can be more easily unit tested.



##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")
+          boolean followAliases,
+      @QueryParam("asyncId") String asyncId)
+      throws Exception {
+
+    final CreateSnapshotResponse response = instantiateJerseyResponse(CreateSnapshotResponse.class);
+    final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+    recordCollectionForLogAndTracing(collName, solrQueryRequest);
+
+    final String collectionName =
+        followAliases
+            ? coreContainer
+                .getZkController()
+                .getZkStateReader()
+                .getAliases()
+                .resolveSimpleAlias(collName)
+            : collName;
+
+    final ClusterState clusterState = coreContainer.getZkController().getClusterState();
+    if (!clusterState.hasCollection(collectionName)) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Collection '" + collectionName + "' does not exist, no action taken.");
+    }
+
+    final SolrZkClient client = coreContainer.getZkController().getZkClient();
+    if (SolrSnapshotManager.snapshotExists(client, collectionName, snapshotName)) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Snapshot with name '"
+              + snapshotName
+              + "' already exists for collection '"
+              + collectionName
+              + "', no action taken.");
+    }
+
+    final ZkNodeProps remoteMessage =
+        createRemoteMessage(collName, followAliases, snapshotName, asyncId);
+    final SolrResponse remoteResponse =
+        CollectionsHandler.submitCollectionApiCommand(
+            coreContainer,
+            coreContainer.getDistributedCollectionCommandRunner(),
+            remoteMessage,
+            CollectionParams.CollectionAction.CREATESNAPSHOT,
+            DEFAULT_COLLECTION_OP_TIMEOUT);
+
+    if (remoteResponse.getException() != null) {
+      throw remoteResponse.getException();
+    }
+
+    response.collection = collName;
+    response.followAliases = followAliases;
+    response.snapshotName = snapshotName;
+    response.requestId = asyncId;
+
+    return response;
+  }
+
+  /**
+   * The Response for {@link CreateCollectionSnapshotAPI}'s {@link #createSnapshot(String, String,
+   * boolean, String)}
+   */
+  public static class CreateSnapshotResponse extends SolrJerseyResponse {
+    @Schema(description = "The name of the collection.")
+    @JsonProperty(COLLECTION_PROP)
+    String collection;
+
+    @Schema(description = "The name of the snapshot to be created.")
+    @JsonProperty("snapshot")
+    String snapshotName;
+
+    @Schema(description = "A flag that treats the collName parameter as a collection alias.")
+    @JsonProperty("followAliases")
+    boolean followAliases;
+
+    @JsonProperty("requestId")

Review Comment:
   [0] Might be worth extending AsyncJerseyResponse instead of 'SolrJerseyResponse' so that this field doesn't need repeated here.



##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")
+          boolean followAliases,
+      @QueryParam("asyncId") String asyncId)

Review Comment:
   [0] I'm not against changing the name of this parameter from "async" (as it appears in the v1 API) to "asyncId", but if we do it should probably be a change we make uniformly across the board in a separate PR.



-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1764,96 +1767,55 @@ public Map<String, Object> execute(
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
-          String collectionName =
-              followAliases
-                  ? h.coreContainer
-                      .getZkController()
-                      .getZkStateReader()
-                      .getAliases()
-                      .resolveSimpleAlias(extCollectionName)
-                  : extCollectionName;
-          String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          if (SolrSnapshotManager.snapshotExists(client, collectionName, commitName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Snapshot with name '"
-                    + commitName
-                    + "' already exists for collection '"
-                    + collectionName
-                    + "', no action taken.");
-          }
+          final CreateCollectionSnapshotAPI createCollectionSnapshotAPI =
+              new CreateCollectionSnapshotAPI(h.coreContainer, req, rsp);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final CreateCollectionSnapshotAPI.CreateSnapshotResponse createSnapshotResponse =
+              createCollectionSnapshotAPI.createSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, createSnapshotResponse);
+
+          return null;
         }),
     DELETESNAPSHOT_OP(
         DELETESNAPSHOT,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final DeleteCollectionSnapshotAPI deleteCollectionSnapshotAPI =
+              new DeleteCollectionSnapshotAPI(h.coreContainer, req, rsp);
+
+          final DeleteCollectionSnapshotAPI.DeleteSnapshotResponse deleteSnapshotResponse =
+              deleteCollectionSnapshotAPI.deleteSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, deleteSnapshotResponse);
+          return null;
         }),
     LISTSNAPSHOTS_OP(
         LISTSNAPSHOTS,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final ListCollectionSnapshotsAPI listCollectionSnapshotsAPI =
+              new ListCollectionSnapshotsAPI(h.coreContainer, req, rsp);
+
+          final ListCollectionSnapshotsAPI.ListSnapshotsResponse response =
+              listCollectionSnapshotsAPI.listSnapshots(req.getParams().get(COLLECTION_PROP));
 
-          NamedList<Object> snapshots = new NamedList<Object>();
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          Collection<CollectionSnapshotMetaData> m =
-              SolrSnapshotManager.listSnapshots(client, collectionName);
-          for (CollectionSnapshotMetaData meta : m) {
+          NamedList<Object> snapshots = new NamedList<>();
+          for (CollectionSnapshotMetaData meta : response.snapshots.values()) {

Review Comment:
   Gotcha, that makes sense. I went back and uncommented the for loop. CollectionSnapshotMetaData does have a toNamedList method, but its not an overriden method. Would it make sense at some point to create some kind of interface that exposes that method and then have it be used by serializers/MessageWriters? I think it would fall outside the scope of this change, but it could make sense if there were other classes out there that behaved the same way.



-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")

Review Comment:
   I think receiving these as a JSON body makes sense. I believe I used QueryPararms, because I had used them in previous work, so didn't think too much. I do think JSON would be cleaner here, especially if it makes the API more consistent with other parts of the API.



-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")
+          boolean followAliases,
+      @QueryParam("asyncId") String asyncId)

Review Comment:
   I'll leave it "async" to be consistent. I don't think I intended to deviate from other methods. Good catch!



-- 
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] Jdurham2843 commented on pull request #1471: Solr 15737 snapshot collection v2

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

   TODO:
   1. Add javadoc documentation and possibly reference guide documentation.
   2. Add tests for the new Jersey Resources. 


-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1764,96 +1767,55 @@ public Map<String, Object> execute(
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
-          String collectionName =
-              followAliases
-                  ? h.coreContainer
-                      .getZkController()
-                      .getZkStateReader()
-                      .getAliases()
-                      .resolveSimpleAlias(extCollectionName)
-                  : extCollectionName;
-          String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          if (SolrSnapshotManager.snapshotExists(client, collectionName, commitName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Snapshot with name '"
-                    + commitName
-                    + "' already exists for collection '"
-                    + collectionName
-                    + "', no action taken.");
-          }
+          final CreateCollectionSnapshotAPI createCollectionSnapshotAPI =
+              new CreateCollectionSnapshotAPI(h.coreContainer, req, rsp);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final CreateCollectionSnapshotAPI.CreateSnapshotResponse createSnapshotResponse =
+              createCollectionSnapshotAPI.createSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, createSnapshotResponse);
+
+          return null;
         }),
     DELETESNAPSHOT_OP(
         DELETESNAPSHOT,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final DeleteCollectionSnapshotAPI deleteCollectionSnapshotAPI =
+              new DeleteCollectionSnapshotAPI(h.coreContainer, req, rsp);
+
+          final DeleteCollectionSnapshotAPI.DeleteSnapshotResponse deleteSnapshotResponse =
+              deleteCollectionSnapshotAPI.deleteSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, deleteSnapshotResponse);
+          return null;
         }),
     LISTSNAPSHOTS_OP(
         LISTSNAPSHOTS,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final ListCollectionSnapshotsAPI listCollectionSnapshotsAPI =
+              new ListCollectionSnapshotsAPI(h.coreContainer, req, rsp);
+
+          final ListCollectionSnapshotsAPI.ListSnapshotsResponse response =
+              listCollectionSnapshotsAPI.listSnapshots(req.getParams().get(COLLECTION_PROP));
 
-          NamedList<Object> snapshots = new NamedList<Object>();
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          Collection<CollectionSnapshotMetaData> m =
-              SolrSnapshotManager.listSnapshots(client, collectionName);
-          for (CollectionSnapshotMetaData meta : m) {
+          NamedList<Object> snapshots = new NamedList<>();
+          for (CollectionSnapshotMetaData meta : response.snapshots.values()) {

Review Comment:
   Gotcha, that makes sense. I went back and uncommented the for loop. CollectionSnapshotMetaData does have a `toNamedList` method, but its not an overriden method. Would it make sense at some point to create some kind of interface that exposes that method and then have it be used by serializers/MessageWriters? I think it would fall outside the scope of this change, but it could make sense if there were other classes out there that behaved the same way.



##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1764,96 +1767,55 @@ public Map<String, Object> execute(
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
-          String collectionName =
-              followAliases
-                  ? h.coreContainer
-                      .getZkController()
-                      .getZkStateReader()
-                      .getAliases()
-                      .resolveSimpleAlias(extCollectionName)
-                  : extCollectionName;
-          String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          if (SolrSnapshotManager.snapshotExists(client, collectionName, commitName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Snapshot with name '"
-                    + commitName
-                    + "' already exists for collection '"
-                    + collectionName
-                    + "', no action taken.");
-          }
+          final CreateCollectionSnapshotAPI createCollectionSnapshotAPI =
+              new CreateCollectionSnapshotAPI(h.coreContainer, req, rsp);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final CreateCollectionSnapshotAPI.CreateSnapshotResponse createSnapshotResponse =
+              createCollectionSnapshotAPI.createSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, createSnapshotResponse);
+
+          return null;
         }),
     DELETESNAPSHOT_OP(
         DELETESNAPSHOT,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final DeleteCollectionSnapshotAPI deleteCollectionSnapshotAPI =
+              new DeleteCollectionSnapshotAPI(h.coreContainer, req, rsp);
+
+          final DeleteCollectionSnapshotAPI.DeleteSnapshotResponse deleteSnapshotResponse =
+              deleteCollectionSnapshotAPI.deleteSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, deleteSnapshotResponse);
+          return null;
         }),
     LISTSNAPSHOTS_OP(
         LISTSNAPSHOTS,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final ListCollectionSnapshotsAPI listCollectionSnapshotsAPI =
+              new ListCollectionSnapshotsAPI(h.coreContainer, req, rsp);
+
+          final ListCollectionSnapshotsAPI.ListSnapshotsResponse response =
+              listCollectionSnapshotsAPI.listSnapshots(req.getParams().get(COLLECTION_PROP));
 
-          NamedList<Object> snapshots = new NamedList<Object>();
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          Collection<CollectionSnapshotMetaData> m =
-              SolrSnapshotManager.listSnapshots(client, collectionName);
-          for (CollectionSnapshotMetaData meta : m) {
+          NamedList<Object> snapshots = new NamedList<>();
+          for (CollectionSnapshotMetaData meta : response.snapshots.values()) {

Review Comment:
   Gotcha, that makes sense. I went back and uncommented the for loop. `CollectionSnapshotMetaData` does have a `toNamedList` method, but its not an overriden method. Would it make sense at some point to create some kind of interface that exposes that method and then have it be used by serializers/MessageWriters? I think it would fall outside the scope of this change, but it could make sense if there were other classes out there that behaved the same way.



-- 
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] Jdurham2843 commented on pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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

   Collection-management.adoc has been updated with three sections on CREATESNAPSHOT, LISTSNAPSHOTS, and DELETESNAPSHOT. I think it's a good idea to do the backup-restore.adoc separately, since it wouldn't be nearly as simple to update and would need to consider other APIs aside from the three worked on here. Let me know what you think!
   
   Regarding the force push: sorry about that! I needed to get some code in from the Apache Solr main branch, so I did a rebase and force push. That's normally the approach I reach for when needing to pull in changes, and can't do a fast-forward. Is there a another way to pull in changes, without needing to force push? My git-fu is fairly rudimentary


-- 
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 #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,162 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.AsyncJerseyResponse;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT) */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @RequestBody(description = "Contains user provided parameters", required = true)
+          CreateSnapshotRequestBody requestBody)
+      throws Exception {
+
+    final CreateSnapshotResponse response = instantiateJerseyResponse(CreateSnapshotResponse.class);
+    final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+    recordCollectionForLogAndTracing(collName, solrQueryRequest);
+
+    final String collectionName = resolveCollectionName(collName, requestBody.followAliases);
+
+    final SolrZkClient client = coreContainer.getZkController().getZkClient();
+    if (SolrSnapshotManager.snapshotExists(client, collectionName, snapshotName)) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Snapshot with name '"
+              + snapshotName
+              + "' already exists for collection '"
+              + collectionName
+              + "', no action taken.");
+    }
+
+    final ZkNodeProps remoteMessage =
+        createRemoteMessage(collName, requestBody.followAliases, snapshotName, requestBody.asyncId);
+    final SolrResponse remoteResponse =
+        CollectionsHandler.submitCollectionApiCommand(
+            coreContainer,
+            coreContainer.getDistributedCollectionCommandRunner(),
+            remoteMessage,
+            CollectionParams.CollectionAction.CREATESNAPSHOT,
+            DEFAULT_COLLECTION_OP_TIMEOUT);
+
+    if (remoteResponse.getException() != null) {
+      throw remoteResponse.getException();
+    }
+
+    response.collection = collName;

Review Comment:
   [0] Just noticed the values being added to the response here.  In general, I've been trying to change the response format as little as possible in these JAX-RS conversion PRs, and this is a bit of a departure from the existing v1 format.
   
   That said, I think it's a change for the better and it's purely additive so it's not a backcompat concern.  So let's go with it in this case.



-- 
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] Jdurham2843 commented on pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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

   Hi @gerlowskija - I believe I've addressed the concerns listed above along with adding some unit tests that cover the Create and Delete collection snapshot endpoints. You mentioned in the jira ticket comments that the actual creation and deletion of snapshots was tested elsewhere, so I think just supplying unit tests may be all that's needed here. Spinning up a full cloud test case may be a little overkill!
   
   I'm still looking into the documentation portion of this work. It may be worth adding the API docs to collection-management.adoc like you mentioned, but I'll need to read through the backup-restore.adoc to see how I'd want to work them into there.


-- 
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] Jdurham2843 commented on pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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

   I'm doing a little digging into this serialization issue, and it looks like we are hitting the else block (line 108) of org.apache.solr.common.util.TextWriter when trying to write CollectionSnapshotMetadata. From looking at the code leading up to this point, I'm starting to think that the JSON message writer for the V1 API assumes that any "objects" it sees are NamedLists or SimpleOrderedMaps. I'm not sure if that's correct or not, but that's my current assumption


-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */

Review Comment:
   Lazy copy-paste on my end, will address



-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */
+  @POST
+  @Path("/{snapshotName}")
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public CreateSnapshotResponse createSnapshot(
+      @Parameter(description = "The name of the collection.", required = true)
+          @PathParam("collName")
+          String collName,
+      @Parameter(description = "The name of the snapshot to be created.", required = true)
+          @PathParam("snapshotName")
+          String snapshotName,
+      @Parameter(description = "A flag that treats the collName parameter as a collection alias.")
+          @DefaultValue("false")
+          @QueryParam("followAliases")
+          boolean followAliases,
+      @QueryParam("asyncId") String asyncId)
+      throws Exception {
+
+    final CreateSnapshotResponse response = instantiateJerseyResponse(CreateSnapshotResponse.class);
+    final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+    recordCollectionForLogAndTracing(collName, solrQueryRequest);
+
+    final String collectionName =
+        followAliases
+            ? coreContainer
+                .getZkController()
+                .getZkStateReader()
+                .getAliases()
+                .resolveSimpleAlias(collName)
+            : collName;
+
+    final ClusterState clusterState = coreContainer.getZkController().getClusterState();
+    if (!clusterState.hasCollection(collectionName)) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Collection '" + collectionName + "' does not exist, no action taken.");
+    }

Review Comment:
   I like this idea, will do!



-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */

Review Comment:
   Lazy copy-paste on my end, will address
   
   (Did you leave off the closing paren on your last line above on purpose?) :) 



-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1764,96 +1767,55 @@ public Map<String, Object> execute(
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
-          String collectionName =
-              followAliases
-                  ? h.coreContainer
-                      .getZkController()
-                      .getZkStateReader()
-                      .getAliases()
-                      .resolveSimpleAlias(extCollectionName)
-                  : extCollectionName;
-          String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          if (SolrSnapshotManager.snapshotExists(client, collectionName, commitName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Snapshot with name '"
-                    + commitName
-                    + "' already exists for collection '"
-                    + collectionName
-                    + "', no action taken.");
-          }
+          final CreateCollectionSnapshotAPI createCollectionSnapshotAPI =
+              new CreateCollectionSnapshotAPI(h.coreContainer, req, rsp);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final CreateCollectionSnapshotAPI.CreateSnapshotResponse createSnapshotResponse =
+              createCollectionSnapshotAPI.createSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, createSnapshotResponse);
+
+          return null;
         }),
     DELETESNAPSHOT_OP(
         DELETESNAPSHOT,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final DeleteCollectionSnapshotAPI deleteCollectionSnapshotAPI =
+              new DeleteCollectionSnapshotAPI(h.coreContainer, req, rsp);
+
+          final DeleteCollectionSnapshotAPI.DeleteSnapshotResponse deleteSnapshotResponse =
+              deleteCollectionSnapshotAPI.deleteSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, deleteSnapshotResponse);
+          return null;
         }),
     LISTSNAPSHOTS_OP(
         LISTSNAPSHOTS,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final ListCollectionSnapshotsAPI listCollectionSnapshotsAPI =
+              new ListCollectionSnapshotsAPI(h.coreContainer, req, rsp);
+
+          final ListCollectionSnapshotsAPI.ListSnapshotsResponse response =
+              listCollectionSnapshotsAPI.listSnapshots(req.getParams().get(COLLECTION_PROP));
 
-          NamedList<Object> snapshots = new NamedList<Object>();
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          Collection<CollectionSnapshotMetaData> m =
-              SolrSnapshotManager.listSnapshots(client, collectionName);
-          for (CollectionSnapshotMetaData meta : m) {
+          NamedList<Object> snapshots = new NamedList<>();
+          for (CollectionSnapshotMetaData meta : response.snapshots.values()) {

Review Comment:
   I used the approach above, because when I tried just squashing, the output I got back wouldn't fully serialize. I essentially would get this back:
   ```{
   	"responseHeader": {
   		"status": 0,
   		"QTime": 14
   	},
   	"snapshots": {
   		"snapshot2": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@24e1513a",
   		"snapshot1": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@50d291df",
   		"snapshot9": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@55cf09d4",
   		"snapshot8": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@542b33e3",
   		"snapshot7": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@7a6dd0dd",
   		"snapshot6": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@3ef5ea26",
   		"snapshot5": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@13d9d585",
   		"snapshot10": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@1e604836"
   	}
   }
   ```
   
   At the time, I couldn't figure out where to make changes to address the serialization problem, so I just sidestepped it. I'd be happy to take another stab at it though!



-- 
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] Jdurham2843 commented on a diff in pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1764,96 +1767,55 @@ public Map<String, Object> execute(
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
-          String collectionName =
-              followAliases
-                  ? h.coreContainer
-                      .getZkController()
-                      .getZkStateReader()
-                      .getAliases()
-                      .resolveSimpleAlias(extCollectionName)
-                  : extCollectionName;
-          String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          if (SolrSnapshotManager.snapshotExists(client, collectionName, commitName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Snapshot with name '"
-                    + commitName
-                    + "' already exists for collection '"
-                    + collectionName
-                    + "', no action taken.");
-          }
+          final CreateCollectionSnapshotAPI createCollectionSnapshotAPI =
+              new CreateCollectionSnapshotAPI(h.coreContainer, req, rsp);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final CreateCollectionSnapshotAPI.CreateSnapshotResponse createSnapshotResponse =
+              createCollectionSnapshotAPI.createSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, createSnapshotResponse);
+
+          return null;
         }),
     DELETESNAPSHOT_OP(
         DELETESNAPSHOT,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final DeleteCollectionSnapshotAPI deleteCollectionSnapshotAPI =
+              new DeleteCollectionSnapshotAPI(h.coreContainer, req, rsp);
+
+          final DeleteCollectionSnapshotAPI.DeleteSnapshotResponse deleteSnapshotResponse =
+              deleteCollectionSnapshotAPI.deleteSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, deleteSnapshotResponse);
+          return null;
         }),
     LISTSNAPSHOTS_OP(
         LISTSNAPSHOTS,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final ListCollectionSnapshotsAPI listCollectionSnapshotsAPI =
+              new ListCollectionSnapshotsAPI(h.coreContainer, req, rsp);
+
+          final ListCollectionSnapshotsAPI.ListSnapshotsResponse response =
+              listCollectionSnapshotsAPI.listSnapshots(req.getParams().get(COLLECTION_PROP));
 
-          NamedList<Object> snapshots = new NamedList<Object>();
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          Collection<CollectionSnapshotMetaData> m =
-              SolrSnapshotManager.listSnapshots(client, collectionName);
-          for (CollectionSnapshotMetaData meta : m) {
+          NamedList<Object> snapshots = new NamedList<>();
+          for (CollectionSnapshotMetaData meta : response.snapshots.values()) {

Review Comment:
   I used the approach above, because when I tried just squashing, the output I got back wouldn't fully serialize. I essentially would get this back:
   ```{
   	"responseHeader": {
   		"status": 0,
   		"QTime": 14
   	},
   	"snapshots": {
   		"snapshot2": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@24e1513a",
   		"snapshot1": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@50d291df",
   		"snapshot9": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@55cf09d4",
   		"snapshot8": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@542b33e3",
   		"snapshot7": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@7a6dd0dd",
   		"snapshot6": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@3ef5ea26",
   		"snapshot5": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@13d9d585",
   		"snapshot10": "org.apache.solr.core.snapshots.CollectionSnapshotMetaData:org.apache.solr.core.snapshots.CollectionSnapshotMetaData@1e604836"
   	}
   }```
   
   At the time, I couldn't figure out where to make changes to address the serialization problem, so I just sidestepped it. I'd be happy to take another stab at it though!



-- 
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 #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionSnapshotAPI.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * 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.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating Collection Snapshots. */
+@Path("/collections/{collName}/snapshots")
+public class CreateCollectionSnapshotAPI extends AdminAPIBase {
+
+  @Inject
+  public CreateCollectionSnapshotAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  /** This API is analogous to V1's (POST /solr/admin/collections?action=CREATESNAPSHOT */

Review Comment:
   Yeah, a bit of a bad joke 😬   Glad you caught it though!



-- 
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 #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1764,96 +1767,55 @@ public Map<String, Object> execute(
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
-          String collectionName =
-              followAliases
-                  ? h.coreContainer
-                      .getZkController()
-                      .getZkStateReader()
-                      .getAliases()
-                      .resolveSimpleAlias(extCollectionName)
-                  : extCollectionName;
-          String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          if (SolrSnapshotManager.snapshotExists(client, collectionName, commitName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Snapshot with name '"
-                    + commitName
-                    + "' already exists for collection '"
-                    + collectionName
-                    + "', no action taken.");
-          }
+          final CreateCollectionSnapshotAPI createCollectionSnapshotAPI =
+              new CreateCollectionSnapshotAPI(h.coreContainer, req, rsp);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final CreateCollectionSnapshotAPI.CreateSnapshotResponse createSnapshotResponse =
+              createCollectionSnapshotAPI.createSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, createSnapshotResponse);
+
+          return null;
         }),
     DELETESNAPSHOT_OP(
         DELETESNAPSHOT,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP, CoreAdminParams.COMMIT_NAME);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final String extCollectionName = req.getParams().get(COLLECTION_PROP);
+          final String commitName = req.getParams().get(CoreAdminParams.COMMIT_NAME);
+          final boolean followAliases = req.getParams().getBool(FOLLOW_ALIASES, false);
+          final String asyncId = req.getParams().get(ASYNC);
 
-          Map<String, Object> params =
-              copy(
-                  req.getParams(),
-                  null,
-                  COLLECTION_PROP,
-                  FOLLOW_ALIASES,
-                  CoreAdminParams.COMMIT_NAME);
-          return params;
+          final DeleteCollectionSnapshotAPI deleteCollectionSnapshotAPI =
+              new DeleteCollectionSnapshotAPI(h.coreContainer, req, rsp);
+
+          final DeleteCollectionSnapshotAPI.DeleteSnapshotResponse deleteSnapshotResponse =
+              deleteCollectionSnapshotAPI.deleteSnapshot(
+                  extCollectionName, commitName, followAliases, asyncId);
+
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, deleteSnapshotResponse);
+          return null;
         }),
     LISTSNAPSHOTS_OP(
         LISTSNAPSHOTS,
         (req, rsp, h) -> {
           req.getParams().required().check(COLLECTION_PROP);
 
-          String extCollectionName = req.getParams().get(COLLECTION_PROP);
-          String collectionName =
-              h.coreContainer
-                  .getZkController()
-                  .getZkStateReader()
-                  .getAliases()
-                  .resolveSimpleAlias(extCollectionName);
-          ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-          if (!clusterState.hasCollection(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' does not exist, no action taken.");
-          }
+          final ListCollectionSnapshotsAPI listCollectionSnapshotsAPI =
+              new ListCollectionSnapshotsAPI(h.coreContainer, req, rsp);
+
+          final ListCollectionSnapshotsAPI.ListSnapshotsResponse response =
+              listCollectionSnapshotsAPI.listSnapshots(req.getParams().get(COLLECTION_PROP));
 
-          NamedList<Object> snapshots = new NamedList<Object>();
-          SolrZkClient client = h.coreContainer.getZkController().getZkClient();
-          Collection<CollectionSnapshotMetaData> m =
-              SolrSnapshotManager.listSnapshots(client, collectionName);
-          for (CollectionSnapshotMetaData meta : m) {
+          NamedList<Object> snapshots = new NamedList<>();
+          for (CollectionSnapshotMetaData meta : response.snapshots.values()) {

Review Comment:
   Ah, ok.  Actually this makes complete sense.  The "squash" pattern only knows how to serialize particular types and interfaces, and I guess CollectionSnapshotMetaData isn't one of those.  Forget I said anything, as long as the output looks correct 👍 



-- 
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 #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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

   New unit tests and other tweaks look great!  We're just down to the docs at this point I think.
   
   > It may be worth adding the API docs to collection-management.adoc like you mentioned, but I'll need to read through the backup-restore.adoc to see how I'd want to work them into there.
   
   Sounds good.  Thinking on it a bit more, it might make sense to split those pieces up.  Adding coverage to collection-management.adoc would be relatively rote small and rote, so it seems "in scope" for this PR.  backup-restore.adoc probably needs some larger restructing, so I'd be fine creating a separate JIRA ticket for it and leaving it for another day.  But that's just my 2c; happy to handle that however you'd like.
   
   > [Jdurham2843](https://github.com/Jdurham2843) [force-pushed](https://github.com/apache/solr/compare/d4617416981f0b0c1aca102987935686a8e396fe..875d7a7d7831cd16b8437880abec17648769e3d2) the SOLR-15737-snapshot-collection-v2 branch from [d461741 ](https://github.com/apache/solr/commit/d4617416981f0b0c1aca102987935686a8e396fe)to [875d7a7 ](https://github.com/apache/solr/commit/875d7a7d7831cd16b8437880abec17648769e3d2)
   [last week](https://github.com/apache/solr/pull/1471#event-8930236452)
   
   One small nitpick process-wise: please avoid force-pushes if you can.  It makes it slightly harder for others from adding small fixes/improvements to your branch, and it makes things a little harder to review in github by doing weird things to ongoing line-level review comments and nullifying Github's really nice "Diff files since your last review" feature.


-- 
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] Jdurham2843 commented on pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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

   Awesome, and thank you for the review and help on 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] gerlowskija merged pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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


-- 
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] alessandrobenedetti commented on pull request #1471: [SOLR-15737]: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (collection level)

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

   Is there any problem with the CHANGES.txt?
   I see some broken checks here, and a contribution from a colleague of mine seems to have a broken crave build caused by the CHANGES.txt ?
   
   Still investigating, so I am sorry if I said any nonsense!


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