Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2679415513 > It has zero consequences for any caller. It's not really about consequences, it's about being consistent/intuitive. Even users who know about the `PATCH` verb and its semantics will probably reach for `PUT` if they're already used to using it on update-field, and update-config, and update-collprop, and update-configset-file, and update-clusterprop, and update-filestore-entry, and update-clusterprop, and update-noderole, and so on. But like I said - this isn't a hill I'm willing to die on: if anyone else votes for PATCH to tilt the cumulative consensus, or we find another API where it'd work, I'm happy to go that route. In the meantime I'm going to try out the "raw-Map" approach and follow up on SOLR-13271 regarding whether we need to support the arbitrary properties now that we have "Collection Props" as a more formalized 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
dsmiley commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2605292284 If Jackson can't handle it then I think it's better to drop the POJO so we can have a better HTTP API. I'll look for more PATCH opportunities but it's not clear to me what problem there would be if only one API used PATCH. It has zero consequences for any caller. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on PR #2930:
URL: https://github.com/apache/solr/pull/2930#issuecomment-2592944289
> Let's just do PATCH to /api/collections/myCollName to update anything
updatable.
We discussed PATCH a bit
[here](https://github.com/apache/solr/pull/2930#discussion_r1915243956), and
the slightly preferred option seemed to be to utilize PATCH only if we can find
other APIs where it makes sense as well. Otherwise, it'd be the only place the
HTTP method is used by Solr, and slightly unintuitive for that reason.
I don't care strongly and am happy to flip-flop if other folks chime in for
PATCH, but I think PUT has more support at the moment (I'm counting
[epugh](https://github.com/apache/solr/pull/2930#discussion_r1907133948) and
[hputman](https://docs.google.com/spreadsheets/d/1HAoBBFPpSiT8mJmgNZKkZAPwfCfPvlc08m5jz3fQBpA/edit?disco=bSSbh4A)).
> to unset a value, why not just use null?
As the PR exists today, with a POJO representing the request body, Jackson
has no way to differentiate between fields that were absent from the request
body entirely, and fields that were present with a null-value. In other words:
if you're cramming them into a POJO at deserialization time, `{"foo": null}`
and `{}` are indistinguishable on the server side.
That said, I guess we don't strictly need a POJO. I _think_ we'd be able to
distinguish these cases if we had Jackson deserialize into a Map instead. It'd
make our OAS and codegen artifacts less specific (and therefore less useful)
for this API, but maybe we're OK with that if it means avoiding the need for a
`DELETE /api/collections/collName/metadata/replicationFactor`.
If folks are OK dropping the request-body POJO, I can investigate going this
route?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2592840832 > Worth some digging there to see what the original intention was, and whether it's worth maintaining both options (at least in terms of the arbitrary/user-specified "modify" properties). The history here is actually kindof interesting. "modify collection" was added back in 6.0, but didn't initially support arbitrary properties. Arbitrary props were added later, and kindof by accident. SOLR-13271 added the "readOnly" property, and the first draft of that functionality added "readOnly" using the syntax that we now call arbitrary/user-provided. The patch later pivoted to handling "readOnly" as a first-class property, similarly to replicationFactor, collection.configName, etc, but the plumbing for "user-provided" props was never removed. Collection properties (added by SOLR-11960) had existed for around a year at this time, but they weren't used very widely, and never came up in the SOLR-13271 discussion. Given all this, I suspect the duplication was accidental oversight and not an intentional choice. Going to ping SOLR-13271 to confirm this, and if so maybe we could deprecate the arbitrary-props in modify-collection in favor of COLLECTIONPROPs. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
dsmiley commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2591540203 +1 on "metadata" vs "properties" for nomenclature. I was thinking about the earlier conundrum a bit. Let's just do PATCH to `/api/collections/myCollName` to update anything updatable. To unset a value, why not just use null? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2591493830 I like "metadata" from a naming perspective. > I do not think the two are as separate as we would like them to be. Yeah, agreed. Worth some digging there to see what the original intention was, and whether it's worth maintaining both options (at least in terms of the arbitrary/user-specified "modify" properties). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
HoustonPutman commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2591063358 > Personally, I'm worried the API design issues highlight deeper confusion in the underlying functionality. Why do we support arbitrary properties in "modify" that are somehow different from "collectionprops". When would a user choose to use one or the other? I don't see an answer in our docs, and I don't know myself 😕 ) Yeah, this is very confusing to me. If the two have to be separate, maybe metadata is a better name for it, and we go with "Collection Properties" and "Collection Metadata", though I do not think the two are as separate as we would like them to be. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on PR #2930:
URL: https://github.com/apache/solr/pull/2930#issuecomment-2590915285
Ugh, actually, using an HTTP DELETE would conflict with existing paths.
- `DELETE /api/collections/someCollName/properties/somePropName` is my first
choice, but that conflicts with an existing API for deleting
collection-properties.
- `DELETE /api/collections/someCollName/somePropName` was my next thought,
but this also conflicts since "properties" is a valid prop name ("modify"
allows users to specify arbitrary props)
We could come up with some new path-segment to differentiate "modify"
properties from "collection props", say, `DELETE
/api/collections/someCollName/collStateProperties/somePropName`, but that feels
hacky? idk.
Personally, I'm worried the API design issues highlight deeper confusion in
the underlying functionality. Why do we support arbitrary properties in
"modify" that are somehow different from "collectionprops". When would a user
choose to use one or the other? I don't see an answer in our docs, and I don't
know myself 😕 )
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2590552508 > Perhaps HTTP DELETE for deleting them one at a time and we see if this is good enough for folks? OK, good call - I'll pursue that route for now. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on code in PR #2930:
URL: https://github.com/apache/solr/pull/2930#discussion_r1915243956
##
solr/api/src/java/org/apache/solr/client/api/endpoint/CollectionApis.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.endpoint;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PUT;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import org.apache.solr.client.api.model.CreateCollectionRequestBody;
+import org.apache.solr.client.api.model.ListCollectionsResponse;
+import org.apache.solr.client.api.model.ModifyCollectionRequestBody;
+import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;
+
+/** V2 API definitions for manipulating Solr collections */
+public interface CollectionApis {
+
+ @Path("/collections")
+ interface List {
+@GET
+@Operation(
+summary = "List all collections in this Solr cluster",
+tags = {"collections"})
+ListCollectionsResponse listCollections();
+ }
+
+ /** V2 API definition for creating a SolrCloud collection */
+ @Path("/collections")
+ interface Create {
+@POST
+@Operation(
+summary = "Creates a new SolrCloud collection.",
+tags = {"collections"})
+SubResponseAccumulatingJerseyResponse
createCollection(CreateCollectionRequestBody requestBody)
+throws Exception;
+ }
+
+ @Path("/collections/{collectionName}")
+ interface Modify {
+@PUT
Review Comment:
> I think that if we embrace PATCH only here, then yeah, it will be kind of
obsurcure. But if we use it on multiple APIs, then I think it's perfectly fine.
That seems like a reasonable line to walk here. Anyone have other APIs in
mind that use similar partial-update semantics? If so, I'll switch this to
'PATCH'. Otherwise I'll stick with 'PUT' for now.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
dsmiley commented on code in PR #2930:
URL: https://github.com/apache/solr/pull/2930#discussion_r1907898100
##
solr/api/src/java/org/apache/solr/client/api/endpoint/CollectionApis.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.endpoint;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PUT;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import org.apache.solr.client.api.model.CreateCollectionRequestBody;
+import org.apache.solr.client.api.model.ListCollectionsResponse;
+import org.apache.solr.client.api.model.ModifyCollectionRequestBody;
+import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;
+
+/** V2 API definitions for manipulating Solr collections */
+public interface CollectionApis {
+
+ @Path("/collections")
+ interface List {
+@GET
+@Operation(
+summary = "List all collections in this Solr cluster",
+tags = {"collections"})
+ListCollectionsResponse listCollections();
+ }
+
+ /** V2 API definition for creating a SolrCloud collection */
+ @Path("/collections")
+ interface Create {
+@POST
+@Operation(
+summary = "Creates a new SolrCloud collection.",
+tags = {"collections"})
+SubResponseAccumulatingJerseyResponse
createCollection(CreateCollectionRequestBody requestBody)
+throws Exception;
+ }
+
+ @Path("/collections/{collectionName}")
+ interface Modify {
+@PUT
Review Comment:
[PATCH](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PATCH)
makes total sense to me. I don't care about relative obscurity.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
epugh commented on code in PR #2930:
URL: https://github.com/apache/solr/pull/2930#discussion_r1907133948
##
solr/api/src/java/org/apache/solr/client/api/endpoint/CollectionApis.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.endpoint;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PUT;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import org.apache.solr.client.api.model.CreateCollectionRequestBody;
+import org.apache.solr.client.api.model.ListCollectionsResponse;
+import org.apache.solr.client.api.model.ModifyCollectionRequestBody;
+import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;
+
+/** V2 API definitions for manipulating Solr collections */
+public interface CollectionApis {
+
+ @Path("/collections")
+ interface List {
+@GET
+@Operation(
+summary = "List all collections in this Solr cluster",
+tags = {"collections"})
+ListCollectionsResponse listCollections();
+ }
+
+ /** V2 API definition for creating a SolrCloud collection */
+ @Path("/collections")
+ interface Create {
+@POST
+@Operation(
+summary = "Creates a new SolrCloud collection.",
+tags = {"collections"})
+SubResponseAccumulatingJerseyResponse
createCollection(CreateCollectionRequestBody requestBody)
+throws Exception;
+ }
+
+ @Path("/collections/{collectionName}")
+ interface Modify {
+@PUT
Review Comment:
I've used PATCH in other contexts.I *think* that if we embrace PATCH
only here, then yeah, it will be kind of obsurcure. But if we use it on
multiple APIs, then I think it's perfectly fine.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on code in PR #2930:
URL: https://github.com/apache/solr/pull/2930#discussion_r1907069757
##
solr/api/src/java/org/apache/solr/client/api/endpoint/CollectionApis.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.endpoint;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PUT;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import org.apache.solr.client.api.model.CreateCollectionRequestBody;
+import org.apache.solr.client.api.model.ListCollectionsResponse;
+import org.apache.solr.client.api.model.ModifyCollectionRequestBody;
+import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;
+
+/** V2 API definitions for manipulating Solr collections */
+public interface CollectionApis {
+
+ @Path("/collections")
+ interface List {
+@GET
+@Operation(
+summary = "List all collections in this Solr cluster",
+tags = {"collections"})
+ListCollectionsResponse listCollections();
+ }
+
+ /** V2 API definition for creating a SolrCloud collection */
+ @Path("/collections")
+ interface Create {
+@POST
+@Operation(
+summary = "Creates a new SolrCloud collection.",
+tags = {"collections"})
+SubResponseAccumulatingJerseyResponse
createCollection(CreateCollectionRequestBody requestBody)
+throws Exception;
+ }
+
+ @Path("/collections/{collectionName}")
+ interface Modify {
+@PUT
Review Comment:
Elsewhere I've proposed using the `PATCH` verb for cases like this -
seemingly it's made for these sort of partial or differential updates. But
I've gotten feedback in the past that it's unintuitive due to its obscurity.
Wdyt?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on code in PR #2930:
URL: https://github.com/apache/solr/pull/2930#discussion_r1907069757
##
solr/api/src/java/org/apache/solr/client/api/endpoint/CollectionApis.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.endpoint;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PUT;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import org.apache.solr.client.api.model.CreateCollectionRequestBody;
+import org.apache.solr.client.api.model.ListCollectionsResponse;
+import org.apache.solr.client.api.model.ModifyCollectionRequestBody;
+import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;
+
+/** V2 API definitions for manipulating Solr collections */
+public interface CollectionApis {
+
+ @Path("/collections")
+ interface List {
+@GET
+@Operation(
+summary = "List all collections in this Solr cluster",
+tags = {"collections"})
+ListCollectionsResponse listCollections();
+ }
+
+ /** V2 API definition for creating a SolrCloud collection */
+ @Path("/collections")
+ interface Create {
+@POST
+@Operation(
+summary = "Creates a new SolrCloud collection.",
+tags = {"collections"})
+SubResponseAccumulatingJerseyResponse
createCollection(CreateCollectionRequestBody requestBody)
+throws Exception;
+ }
+
+ @Path("/collections/{collectionName}")
+ interface Modify {
+@PUT
Review Comment:
I've proposed using the `PATCH` verb for cases like this - seemingly it's
made for these sort of partial or differential updates. But I've gotten
feedback in the past that it's unintuitive due to its obscurity. Wdyt?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on code in PR #2930:
URL: https://github.com/apache/solr/pull/2930#discussion_r1907069757
##
solr/api/src/java/org/apache/solr/client/api/endpoint/CollectionApis.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.endpoint;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PUT;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import org.apache.solr.client.api.model.CreateCollectionRequestBody;
+import org.apache.solr.client.api.model.ListCollectionsResponse;
+import org.apache.solr.client.api.model.ModifyCollectionRequestBody;
+import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;
+
+/** V2 API definitions for manipulating Solr collections */
+public interface CollectionApis {
+
+ @Path("/collections")
+ interface List {
+@GET
+@Operation(
+summary = "List all collections in this Solr cluster",
+tags = {"collections"})
+ListCollectionsResponse listCollections();
+ }
+
+ /** V2 API definition for creating a SolrCloud collection */
+ @Path("/collections")
+ interface Create {
+@POST
+@Operation(
+summary = "Creates a new SolrCloud collection.",
+tags = {"collections"})
+SubResponseAccumulatingJerseyResponse
createCollection(CreateCollectionRequestBody requestBody)
+throws Exception;
+ }
+
+ @Path("/collections/{collectionName}")
+ interface Modify {
+@PUT
Review Comment:
I've proposed using the `PATCH` verb for cases like this - seemingly it's
made for cases like this. But I've gotten feedback in the past that it's
unintuitive due to its obscurity. Wdyt?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
dsmiley commented on code in PR #2930:
URL: https://github.com/apache/solr/pull/2930#discussion_r1902204246
##
solr/api/src/java/org/apache/solr/client/api/endpoint/CollectionApis.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.endpoint;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PUT;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import org.apache.solr.client.api.model.CreateCollectionRequestBody;
+import org.apache.solr.client.api.model.ListCollectionsResponse;
+import org.apache.solr.client.api.model.ModifyCollectionRequestBody;
+import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;
+
+/** V2 API definitions for manipulating Solr collections */
+public interface CollectionApis {
+
+ @Path("/collections")
+ interface List {
+@GET
+@Operation(
+summary = "List all collections in this Solr cluster",
+tags = {"collections"})
+ListCollectionsResponse listCollections();
+ }
+
+ /** V2 API definition for creating a SolrCloud collection */
+ @Path("/collections")
+ interface Create {
+@POST
+@Operation(
+summary = "Creates a new SolrCloud collection.",
+tags = {"collections"})
+SubResponseAccumulatingJerseyResponse
createCollection(CreateCollectionRequestBody requestBody)
+throws Exception;
+ }
+
+ @Path("/collections/{collectionName}")
+ interface Modify {
+@PUT
Review Comment:
BTW I'm suspicious of your proposed use of HTTP **PUT** as the payload isn't
an entire representation of the target resource. It modifies an existing one
that may have characteristics specified on creation that are not here. PUT
doesn't have much applicability.
[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT
"**replaces** a representation of the target resource with the request content"
[2] https://restfulapi.net/rest-put-vs-post/ "PUT **replaces** the resource
in its entirety"
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
dsmiley commented on code in PR #2930:
URL: https://github.com/apache/solr/pull/2930#discussion_r1902204246
##
solr/api/src/java/org/apache/solr/client/api/endpoint/CollectionApis.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.endpoint;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import jakarta.ws.rs.DELETE;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PUT;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.QueryParam;
+import org.apache.solr.client.api.model.CreateCollectionRequestBody;
+import org.apache.solr.client.api.model.ListCollectionsResponse;
+import org.apache.solr.client.api.model.ModifyCollectionRequestBody;
+import org.apache.solr.client.api.model.SubResponseAccumulatingJerseyResponse;
+
+/** V2 API definitions for manipulating Solr collections */
+public interface CollectionApis {
+
+ @Path("/collections")
+ interface List {
+@GET
+@Operation(
+summary = "List all collections in this Solr cluster",
+tags = {"collections"})
+ListCollectionsResponse listCollections();
+ }
+
+ /** V2 API definition for creating a SolrCloud collection */
+ @Path("/collections")
+ interface Create {
+@POST
+@Operation(
+summary = "Creates a new SolrCloud collection.",
+tags = {"collections"})
+SubResponseAccumulatingJerseyResponse
createCollection(CreateCollectionRequestBody requestBody)
+throws Exception;
+ }
+
+ @Path("/collections/{collectionName}")
+ interface Modify {
+@PUT
Review Comment:
BTW I'm suspicious of your proposed use of HTTP **PUT** as the payload isn't
an entire representation of the target resource. It modifies an existing one
that may have characteristics specified on creation that are not here. PUT
doesn't have much applicability.
[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT
"**replaces** a representation of the target resource with the request content"
[1] https://restfulapi.net/rest-put-vs-post/ "PUT **replaces** the resource
in its entirety"
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
epugh commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2569248487 > Perhaps HTTP DELETE for deleting them one at a time and we see if this is good enough for folks? This is what I was thinking as well! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
dsmiley commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2568689273 Perhaps HTTP DELETE for deleting them one at a time and we see if this is good enough for folks? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-16391: Convert "modify-coll" API to JAX-RS [solr]
gerlowskija commented on PR #2930: URL: https://github.com/apache/solr/pull/2930#issuecomment-2566985693 The biggest open question for this API (IMO) is how the "property unsetting" semantics should work for the v2 API. For context, the v1 API has "patch" semantics. That is - only the properties being modified need be provided. This is convenient and intuitive, but creates a problem that the v1 API felt it needed to solve: how can users "unset" a property entirely? In v1 this is done by providing a property, but with a value of empty-string (i.e. `""`). This is a problem in the v2 world, where properties are strongly typed. The v2 API expects properties like `perReplicaState` or `replicationFactor` to have non-string types (Boolean and Integer, respectively), so the empty-string flag value (understandably) triggers a Jackson deserialization error. I'm not totally sure what to do about this. We've got a few options, but I'm not sure what is best. 1. Ditch the strong-typing on our request-body POJO, so that the empty-string flag val is valid for all properties. 2. Investigate whether Jackson has some way to differentiate which properties were provided in the input JSON, and use that to support "unsetting". 3. Add an additional "unset" property to the request-body POJO, that takes a list of property names to unset in the collection-state. I've taken this approach for now in the PR to get tests, etc. passing - it was simple and easy to implement. But I don't like it as a long term solution. 4. Ditch support for property "unsetting" in v2 altogether. Others may know, but I'm not sure what use-case this capability serves. IMO it's confusing semantically - what does it mean to unset a "replicationFactor" property, for instance? It may be around for a very clear and relevant purpose, but if it's not, maybe removing it is a plausible path forward? 5. Something else? I'm a bit stumped on how to proceed, so I'd love feedback or thoughts if anyone has them. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
