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/22 23:55:22 UTC

[GitHub] [solr] epugh opened a new pull request #41: SOLR-11646: document v2 api (WIP)

epugh opened a new pull request #41:
URL: https://github.com/apache/solr/pull/41


   # Description
   
   Continuing to add V2 api documentation.
   
   # Solution
   
   Running `bin/solr -c -e techproducts` and then testing the commands.  Using https://docs.google.com/spreadsheets/d/1d3scMpjxSt5HAURkDHmMhQ7FrFuV0umyJyBT0lV_HX0/edit#gid=654750626 as a reference.
   
   # Tests
   
   This is all docs.
   
   # 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.
   - [ X] 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.

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



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

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on a change in pull request #41:
URL: https://github.com/apache/solr/pull/41#discussion_r616593712



##########
File path: solr/solr-ref-guide/src/replica-management.adoc
##########
@@ -17,14 +17,77 @@
 // specific language governing permissions and limitations
 // under the License.
 
-A replica is a physical copy of a shard.
+A replica is a physical copy of a shard.  Replicas enhance fail over by providing additional copies of the data
+and enhance scalability by providing additional capacity for searching.
+
+The examples assume you have started Solr via `bin/solr start -c -e techproducts`.
 
 [[addreplica]]
 == ADDREPLICA: Add Replica
 
 Add one or more replicas to a shard in a collection. The node name can be specified if the replica is to be created in a specific node. Otherwise, a set of nodes can be specified and the most suitable ones among them will be chosen to create the replica(s).
 
-`/admin/collections?action=ADDREPLICA&collection=_collection_&shard=_shard_&node=_nodeName_`
+[.dynamic-tabs]
+--
+
+[example.tab-pane#v1addreplica]
+====
+[.tab-label]*V1 API*
+
+*Input*
+
+[source,text]
+----
+http://localhost:8983/solr/admin/collections?action=ADDREPLICA&collection=techproducts&shard=shard&node=localhost:8983_solr
+----
+
+*Output*
+
+[source,xml]
+----
+<response>
+  <lst name="responseHeader">
+    <int name="status">0</int>
+    <int name="QTime">122</int>
+  </lst>
+</response>

Review comment:
       [Q] Is there a reason you used XML as the output format here, instead of the default JSON?
   
   I don't particularly care about the inconsistency, but you might want to change the command above to included the `wt=xml` param, so that a novice user wouldn't be wondering why they get a different format when they try this example...

##########
File path: solr/solr-ref-guide/src/shard-management.adoc
##########
@@ -205,7 +261,61 @@ Shards can only created with this API for collections that use the 'implicit' ro
 
 Use SPLITSHARD for collections created with the 'compositeId' router (`router.key=compositeId`).
 
-`/admin/collections?action=CREATESHARD&shard=_shardName_&collection=_name_`
+[.dynamic-tabs]
+--
+
+[example.tab-pane#v1createshard]
+====
+[.tab-label]*V1 API*
+
+*Input*
+
+[source,text]
+----
+http://localhost:8983/solr/admin/collections?action=CREATESHARD&shard=_shardName_&collection=_name_

Review comment:
       [0] It might be cool if the v1 and v2 examples here used the same param vals so they were truly equivalent.

##########
File path: solr/solr-ref-guide/src/shard-management.adoc
##########
@@ -334,7 +474,59 @@ http://localhost:8983/solr/admin/collections?action=DELETESHARD&collection=anoth
 
 In the unlikely event of a shard losing its leader, this command can be invoked to force the election of a new leader.
 
-`/admin/collections?action=FORCELEADER&collection=<collectionName>&shard=<shardName>`
+[.dynamic-tabs]
+--
+
+[example.tab-pane#v1forceleader]
+====
+[.tab-label]*V1 API*
+
+*Input*
+
+[source,text]
+----
+http://localhost:8983/solr/admin/collections?action=FORCELEADER&collection=techproducts&shard=shard1
+----
+
+*Output*
+
+[source,xml]
+----
+<response>

Review comment:
       ditto, re: we might want to change this to JSON or add `wt=xml` to the request.

##########
File path: solr/solr-ref-guide/src/replica-management.adoc
##########
@@ -17,14 +17,77 @@
 // specific language governing permissions and limitations
 // under the License.
 
-A replica is a physical copy of a shard.
+A replica is a physical copy of a shard.  Replicas enhance fail over by providing additional copies of the data
+and enhance scalability by providing additional capacity for searching.
+
+The examples assume you have started Solr via `bin/solr start -c -e techproducts`.
 
 [[addreplica]]
 == ADDREPLICA: Add Replica
 
 Add one or more replicas to a shard in a collection. The node name can be specified if the replica is to be created in a specific node. Otherwise, a set of nodes can be specified and the most suitable ones among them will be chosen to create the replica(s).
 
-`/admin/collections?action=ADDREPLICA&collection=_collection_&shard=_shard_&node=_nodeName_`
+[.dynamic-tabs]
+--
+
+[example.tab-pane#v1addreplica]
+====
+[.tab-label]*V1 API*
+
+*Input*
+
+[source,text]
+----
+http://localhost:8983/solr/admin/collections?action=ADDREPLICA&collection=techproducts&shard=shard&node=localhost:8983_solr
+----
+
+*Output*
+
+[source,xml]
+----
+<response>
+  <lst name="responseHeader">
+    <int name="status">0</int>
+    <int name="QTime">122</int>
+  </lst>
+</response>

Review comment:
       Also, I'm surprised the actual response content looks so different from the v2-output for this API below.  Nothing would surprise me, but I thought v1/v2 here were hitting the same underlying implementation.

##########
File path: solr/solr-ref-guide/src/replica-management.adoc
##########
@@ -321,7 +412,42 @@ http://localhost:8983/solr/admin/collections?action=DELETEREPLICA&collection=tes
 
 Assign an arbitrary property to a particular replica and give it the value specified. If the property already exists, it will be overwritten with the new value.
 
-`/admin/collections?action=ADDREPLICAPROP&collection=collectionName&shard=shardName&replica=replicaName&property=propertyName&property.value=value`
+[.dynamic-tabs]
+--
+
+[example.tab-pane#v1addreplicaprop]
+====
+[.tab-label]*V1 API*
+
+*Input*
+
+[source,text]
+----
+http://localhost:8983/solr/admin/collections?action=ADDREPLICAPROP&collection=collectionName&shard=shardName&replica=replicaName&property=propertyName&property.value=value

Review comment:
       [0] It might be cool if the v1 and v2 examples here used the same param vals so they were truly equivalent.

##########
File path: solr/solr-ref-guide/src/replica-management.adoc
##########
@@ -409,7 +535,41 @@ http://localhost:8983/solr/admin/collections?action=ADDREPLICAPROP&shard=shard1&
 
 Deletes an arbitrary property from a particular replica.
 
-`/admin/collections?action=DELETEREPLICAPROP&collection=collectionName&shard=_shardName_&replica=_replicaName_&property=_propertyName_`
+[.dynamic-tabs]
+--
+
+[example.tab-pane#v1deletereplicaprop]
+====
+[.tab-label]*V1 API*
+
+*Input*
+
+[source,text]
+----
+http://localhost:8983/solr/admin/collections?action=DELETEREPLICAPROP&collection=collectionName&shard=_shardName_&replica=_replicaName_&property=_propertyName_

Review comment:
       [0] It might be cool if the v1 and v2 examples here used the same param vals so they were truly equivalent.

##########
File path: solr/solr-ref-guide/src/shard-management.adoc
##########
@@ -21,10 +21,66 @@ In SolrCloud, a shard is a logical partition of a collection. This partition sto
 
 The number of shards you have helps to determine how many documents a single collection can contain in total, and also impacts search performance.
 
+The examples assume you have started Solr via `bin/solr start -c -e techproducts`.
+
 [[splitshard]]
 == SPLITSHARD: Split a Shard
 
-`/admin/collections?action=SPLITSHARD&collection=_name_&shard=_shardID_`
+[.dynamic-tabs]
+--
+
+[example.tab-pane#v1splitshard]
+====
+[.tab-label]*V1 API*
+
+*Input*
+
+[source,text]
+----
+http://localhost:8983/solr/admin/collections?action=SPLITSHARD&collection=techproducts&shard=shard1
+----
+
+*Output*
+
+[source,xml]
+----
+<response>

Review comment:
       [0] ditto, re: might be worth either changing the output to JSON or adding `wt=xml` to the request




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



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


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

Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #41:
URL: https://github.com/apache/solr/pull/41#discussion_r599498952



##########
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:
       In other places on GET's we don't have the "Content-type" header, so removing it.  What do you think about on a DELETE?  Same thing?




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



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

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on a change in pull request #41:
URL: https://github.com/apache/solr/pull/41#discussion_r599683105



##########
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:
       Yeah, that would be really cool.  Now that the pdf is dead it's an actual option for once.




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



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

Posted by GitBox <gi...@apache.org>.
gerlowskija edited a comment on pull request #41:
URL: https://github.com/apache/solr/pull/41#issuecomment-825135620


   I rm'd the stuff that looked redundant to me, so I'm fine w/ merging whenever at this point.  I'll leave it open for the weekend to let you do the honors, or in case you want to make sure I wasn't overzealous in ripping out redundancy, but otherwise I'll click the button on Monday.
   
   Thanks for picking this up Eric


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



---------------------------------------------------------------------
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 #41: SOLR-11646: document v2 api (WIP)

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on pull request #41:
URL: https://github.com/apache/solr/pull/41#issuecomment-823239861


   > Have I missed any other api's that are v2 equivalent?
   
   I think you've covered all the collection-admin APIs at least.


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



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


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

Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #41:
URL: https://github.com/apache/solr/pull/41#discussion_r599498153



##########
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:
       Sorry, this is my fault.   Please see SOLR-15276 / #40.   In the commit I tried to flag that this was depending on another PR.   I should maybe have kept things seperate completely, and not wasted your time!   Lesson learned.   Agreed on the non existant 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.

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



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

Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #41:
URL: https://github.com/apache/solr/pull/41#discussion_r599496557



##########
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:
       It doesn't exist unless you want to give me a LGTM or +1 over on SOLR-15278 / #43 ;-)     Good grammer 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.

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



[GitHub] [solr] epugh merged pull request #41: SOLR-11646: document v2 api (WIP)

Posted by GitBox <gi...@apache.org>.
epugh merged pull request #41:
URL: https://github.com/apache/solr/pull/41


   


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



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


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

Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #41:
URL: https://github.com/apache/solr/pull/41#issuecomment-824766776


   your edits look great!   
   
   WDYT about merging after those last edits?  Feel free to click the magic button!


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



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


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

Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #41:
URL: https://github.com/apache/solr/pull/41#issuecomment-821117831


   > @epugh Can I do anything to help this along? I think it was initially dependent on #40 but with that merged I wasn't sure if this was ready to go or not...
   
   I think you did step one, a gentle nudge ;-)   It's always motivating to know someone else cares about a PR!   Will make progress today....   See if I can't get to another reasonable commit point, and then start another one.


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



---------------------------------------------------------------------
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 #41: SOLR-11646: document v2 api (WIP)

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on pull request #41:
URL: https://github.com/apache/solr/pull/41#issuecomment-824752402


   Hey @epugh I pushed up a few fixes for my previous review comments.
   
   In the process though I noticed something odd.  On certain API calls, the docs still have two sets of examples.  Take `DELETEREPLICA` in `replica-management.adoc` for example.  The section gives a short overview, then has some tabbed v1/v2 examples, then it has detailed description for each API parameter.  All great so far!  But then before the next API command, it gives _more_ DELETEREPLICA examples.
   
   In some cases (e.g. ADDREPLICA) the second set of examples are different enough from the first set that they might provide value.  But in other cases it looks like the two sets of examples are near duplicates and we just forgot to remove them.
   
   Was this something you did thoughtfully?  Or was it just an oversight?  Happy to go through and fix myself - just wanted to make sure I wasn't undoing something you did intentionally.


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



---------------------------------------------------------------------
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 #41: SOLR-11646: document v2 api (WIP)

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on pull request #41:
URL: https://github.com/apache/solr/pull/41#issuecomment-820666938


   @epugh Can I do anything to help this along?  I think it was initially dependent on #40 but with that merged I wasn't sure if this was ready to go or not... 


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



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


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

Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #41:
URL: https://github.com/apache/solr/pull/41#issuecomment-821811819


   okay, I think I am there on this...   Have I missed any other api's that are v2 equivalent?


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



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


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

Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #41:
URL: https://github.com/apache/solr/pull/41#issuecomment-824766011


   > Hey @epugh I pushed up a few fixes for my previous review comments.
   > 
   > In the process though I noticed something odd. On certain API calls, the docs still have two sets of examples. Take `DELETEREPLICA` in `replica-management.adoc` for example. The section gives a short overview, then has some tabbed v1/v2 examples, then it has detailed description for each API parameter. All great so far! But then before the next API command, it gives _more_ DELETEREPLICA examples.
   > 
   > In some cases (e.g. ADDREPLICA) the second set of examples are different enough from the first set that they might provide value. But in other cases it looks like the two sets of examples are near duplicates and we just forgot to remove them.
   > 
   > Was this something you did thoughtfully? Or was it just an oversight? Happy to go through and fix myself - just wanted to make sure I wasn't undoing something you did intentionally.
   
   If you are in there, I think go ahead and delete them!   I think I ended up changing the structure to make the v1 and v2 examples always at the top, and then the more detailed down below, that might have caused some duplicates.  


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



---------------------------------------------------------------------
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 #41: SOLR-11646: document v2 api (WIP)

Posted by GitBox <gi...@apache.org>.
gerlowskija commented on pull request #41:
URL: https://github.com/apache/solr/pull/41#issuecomment-825135620


   I rm'd the stuff that looked redundant to me, so I'm fine w/ merging whenever at this point.  I'll leave it open for the weekend to let you do the honors, or in case you want to make sure I wasn't overzealous in ripping out redundancy, but otherwise I'll click the button on Monday.


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



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


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

Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #41:
URL: https://github.com/apache/solr/pull/41#discussion_r599495520



##########
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:
       ;-)   I think the reason was that the V2 api's use of JSON meant that having the full spelled out curl command was much more useful..    In v1, you just cut'n'paste the URL into your browser and off it goes!
   
   What I'd really like is to take a page from Elasticsearch docs, and instead have a "copy as curl" option that adds all the curl wrapper around the example.  




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



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

Posted by GitBox <gi...@apache.org>.
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