You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/09/29 19:51:36 UTC

[GitHub] [solr] gerlowskija opened a new pull request, #1049: SOLR-16392: Refactor and update v2 ADDREPLICAPROP API

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

   # Description
   
   The Solr community is engaged in an ongoing effort to make Solr's v2 API more REST-ful and intuitive for users so that it can eventually serve as a viable v1 replacement.  This PR in particular focuses on Solr's "add-replica-prop" API, aligning it with the API design described here and agreed to here and here.
   
   In particular this changes the v2 API from:
   
   ```
   POST /collections/collName {"add-replica-property" : {...}}
   ```
   
   to...
   
   ```
   PUT /collections/<coll>/shards/<shard>/replicas/<replica>/properties/<prop> {"value": <val>}
   ```
   
   This is a breaking change for users of the v2 API, but one that is allowed due to v2's "experimental" designation.
   
   # Solution
   
   This PR modifies add-replica-prop in the way described, but it also refactors the API class to use the newly added JAX-RS framework in the process.  This framework-switchover, while conceptually separate from the API realignment in this case, makes sense as both tasks were coming in the short term and touch identical chunks of code.
   
   As a result, this PR not only changes the cosmetics of the add-replica-prop endpoint, it also adds it to Solr's growing OpenAPI spec.
   
   As suggested in the past on similar refactors, I've also moved the logic underlying the add-replica-prop functionality (previously hosted in `CollectionsHandler`) over to the v2 `AddReplicaPropertyAPI` class.  The v1 codepath in `CollectionsHandler` now instantiates an AddReplicaPropertyAPI to do its job.
   
   # Tests
   
   New unit tests for `AddReplicaPropertyAPI`, as well as all the existing v1 and v2 tests for the add-replica-prop functionality continuing to pass.
   
   # 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.
   - [ ] I have run `./gradlew check`.
   - [x] 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] sonatype-lift[bot] commented on pull request #1049: SOLR-16392: Refactor and update v2 ADDREPLICAPROP API

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on PR #1049:
URL: https://github.com/apache/solr/pull/1049#issuecomment-1262792679

   :warning: **314 God Classes** were detected by Lift in this project. [Visit the Lift web console](https://lift.sonatype.com/results/github.com/apache/solr/01GE5CVW98CX7BRCSVGZ6HG99W?tab=technical-debt&utm_source=github.com&utm_campaign=lift-comment&utm_content=apache\%20solr) for more details.


-- 
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] dsmiley commented on a diff in pull request #1049: SOLR-16392: Refactor and update v2 ADDREPLICAPROP API

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1049:
URL: https://github.com/apache/solr/pull/1049#discussion_r991390487


##########
solr/solr-ref-guide/modules/deployment-guide/pages/replica-management.adoc:
##########
@@ -610,14 +610,9 @@ http://localhost:8983/solr/admin/collections?action=ADDREPLICAPROP&collection=te
 
 [source,bash]
 ----
-curl -X POST http://localhost:8983/api/collections/techproducts -H 'Content-Type: application/json' -d '
+curl -X PUT http://localhost:8983/api/collections/techproducts/shards/shard1/replicas/core_node2/properties/preferredLeader -H 'Content-Type: application/json' -d '
   {
-    "add-replica-property":{
-      "shard":"shard1",
-      "replica":"core_node2",
-      "name":"preferredLeader",
-      "value": "true"
-    }
+    "value": "true"
   }

Review Comment:
   @gerlowskija The wrapper JSON seems unnecessary & superfluous for such a simple name=value.  Can't we support just the HTTP payload of "true" ?



-- 
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 #1049: SOLR-16392: Refactor and update v2 ADDREPLICAPROP API

Posted by GitBox <gi...@apache.org>.
gerlowskija merged PR #1049:
URL: https://github.com/apache/solr/pull/1049


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