You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/03/23 01:29:04 UTC

[GitHub] [solr] gerlowskija commented on a change in pull request #41: SOLR-11646: document v2 api (WIP)

gerlowskija commented on a change in pull request #41:
URL: https://github.com/apache/solr/pull/41#discussion_r599174898



##########
File path: solr/solr-ref-guide/src/collections-api.adoc
##########
@@ -66,7 +96,29 @@ http://localhost:8983/solr/admin/collections?action=SPLITSHARD&collection=collec
 
 Request the status and response of an already submitted <<Asynchronous Calls,Asynchronous Collection API>> (below) call. This call is also used to clear up the stored statuses.
 
-`/admin/collections?action=REQUESTSTATUS&requestid=_request-id_`
+[.dynamic-tabs]
+--
+[example.tab-pane#v1asyncrequeststatus]
+====
+[.tab-label]*V1 API*
+
+[source,bash]
+----
+http://localhost:8983/solr/admin/collections?action=REQUESTSTATUS&requestid=1000
+
+----
+====
+
+[example.tab-pane#v2asyncrequeststatus]
+====
+[.tab-label]*V2 API*
+
+[source,bash]
+----
+curl -X GET http://localhost:8983/api/cluster/command-status/1000 -H 'Content-Type: application/json'

Review comment:
       [0] It's not wrong, and I'm fine w/ it as is.  But you could drop the "Content-type" header here if you wanted to go for concise-ness.
   
   [0] I tested this out and ran into a response that differs a bit from what I expected.
   
   ```
   ➜  solr-9.0.0-SNAPSHOT git:(main) curl -ilk "http://localhost:8983/api/cluster/command-status/1000" -H "Content-type: application/json"                                       
   HTTP/1.1 200 OK
   Content-Type: application/json;charset=utf-8
   Content-Length: 81
   
   {
     "responseHeader":{
       "status":0,
       "QTime":0},
     "availableSubPaths":{}}
   ```
   
   Suspicious, I tried out a nonexistent request-id and a malformed (spaces, punctuation, etc. in the ID) request-id and got the exact same response as the one above, which was surprising.  Solr should probably return an error on at least the nonexistent case.
   
   Since all my calls, even clearly invalid ones, spat out the same response, I'm wondering if the API itself is even right.  Did you test this out locally?  Am I overlooking something obvious maybe?
   
   I'll prob file a separate JIRA to get to the bottom of this, unless you see something obvious I'm overlooking in my test?

##########
File path: solr/solr-ref-guide/src/collections-api.adoc
##########
@@ -42,22 +42,52 @@ As of now, REQUESTSTATUS does not automatically clean up the tracking data struc
 
 *Input*
 
+[.dynamic-tabs]
+--
+[example.tab-pane#v1asyncexample]
+====
+[.tab-label]*V1 API*
+
+[source,bash]
+----
+http://localhost:8983/solr/admin/collections?action=SPLITSHARD&collection=collection1&shard=shard1&async=1000

Review comment:
       [0] I mentioned this before, forget what the outcome was.  If you don't care about the consistency, it's fine by me.
   
   But it's unexpected that the v1 snippet is a plain URL, but the v2 API is a full curl command.  The PR is consistent on v1=url, v2=curl from example to example, so I imagine you've got a reason - just curious what it is.

##########
File path: solr/solr-ref-guide/src/collections-api.adoc
##########
@@ -126,7 +178,36 @@ http://localhost:8983/solr/admin/collections?action=REQUESTSTATUS&requestid=1004
 
 Deletes the stored response of an already failed or completed <<Asynchronous Calls,Asynchronous Collection API>> call.
 
-`/admin/collections?action=DELETESTATUS&requestid=_request-id_`
+[.dynamic-tabs]
+--
+[example.tab-pane#v1asyncdeletestatus]
+====
+[.tab-label]*V1 API*
+
+[source,bash]
+----
+http://localhost:8983/solr/admin/collections?action=DELETESTATUS&requestid=1000
+
+----
+====
+
+[example.tab-pane#v2asyncdeletestatus]
+====
+[.tab-label]*V2 API*
+
+Delete a single request response:
+[source,bash]
+----
+curl -X DELETE http://localhost:8983/api/cluster/command-status/1000 -H 'Content-Type: application/json'

Review comment:
       [0] Ditto, re: Content-type is unnecessary on non-POST requests.

##########
File path: solr/solr-ref-guide/src/collections-api.adoc
##########
@@ -126,7 +178,36 @@ http://localhost:8983/solr/admin/collections?action=REQUESTSTATUS&requestid=1004
 
 Deletes the stored response of an already failed or completed <<Asynchronous Calls,Asynchronous Collection API>> call.
 
-`/admin/collections?action=DELETESTATUS&requestid=_request-id_`
+[.dynamic-tabs]
+--
+[example.tab-pane#v1asyncdeletestatus]
+====
+[.tab-label]*V1 API*
+
+[source,bash]
+----
+http://localhost:8983/solr/admin/collections?action=DELETESTATUS&requestid=1000
+
+----
+====
+
+[example.tab-pane#v2asyncdeletestatus]
+====
+[.tab-label]*V2 API*
+
+Delete a single request response:
+[source,bash]
+----
+curl -X DELETE http://localhost:8983/api/cluster/command-status/1000 -H 'Content-Type: application/json'
+----
+
+Flush out all stored completed and failed async request response:
+[source,bash]
+----
+curl -X DELETE http://localhost:8983/api/cluster/command-status -H 'Content-Type: application/json'

Review comment:
       [0] Ditto, re: Content-type is unnecessary on non-POST requests.

##########
File path: solr/solr-ref-guide/src/collections-api.adoc
##########
@@ -126,7 +178,36 @@ http://localhost:8983/solr/admin/collections?action=REQUESTSTATUS&requestid=1004
 
 Deletes the stored response of an already failed or completed <<Asynchronous Calls,Asynchronous Collection API>> call.
 
-`/admin/collections?action=DELETESTATUS&requestid=_request-id_`
+[.dynamic-tabs]
+--
+[example.tab-pane#v1asyncdeletestatus]
+====
+[.tab-label]*V1 API*
+
+[source,bash]
+----
+http://localhost:8983/solr/admin/collections?action=DELETESTATUS&requestid=1000
+
+----
+====
+
+[example.tab-pane#v2asyncdeletestatus]
+====
+[.tab-label]*V2 API*
+
+Delete a single request response:
+[source,bash]
+----
+curl -X DELETE http://localhost:8983/api/cluster/command-status/1000 -H 'Content-Type: application/json'
+----
+
+Flush out all stored completed and failed async request response:

Review comment:
       [0] Should the last word here be plural (i.e. 'responses')?
   
   Also, neat, didn't know this existed.




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

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