gerlowskija commented on code in PR #4264:
URL: https://github.com/apache/solr/pull/4264#discussion_r3081639109
##########
solr/api/src/java/org/apache/solr/client/api/endpoint/ConfigsetsApi.java:
##########
@@ -71,6 +79,46 @@ SolrJerseyResponse
deleteConfigSet(@PathParam("configSetName") String configSetN
throws Exception;
}
+ /**
+ * V2 API definition for downloading an existing configset as a ZIP archive.
+ *
+ * <p>Equivalent to GET /api/configsets/{configSetName}/download
+ */
+ @Path("/configsets/{configSetName}")
+ interface Download {
+ @GET
+ @Path("/download")
Review Comment:
> I was thinking about it and then asked Claude for soem suggestions, and he
kind of came back to
I don't mean to be glib, but I don't really care what Claude thinks - I care
what you think! Maybe by quoting Claude you're implying you agree? But I'd
love to hear which option you think is better, and why!
> If we had another places that we did the zip download, then I could see
making them all work the same
Idk about ZIP downloads, that's admittedly a little more niche 🤷
What **is** clear to me is that we have a bunch of APIs that let you
"download some thing" or "download some part of some thing", and none of those
APIs take the `/download` approach suggested by your LLM.
- Download a file from the filestore: `GET
/api/cluster/filestore/files/<path>`
- Download an index file from leader: `GET
/api/cores/coreName/replication/files/<path>`
- Fetch the data in a ZooKeeper node: `GET
/api/cluster/zookeeper/data/<path>`
- etc.
We've got an established pattern here: rely on the HTTP verb to imply
"download", and offer a "files/<filepath>" sub-path as a way to support
individual file download. If we want to deviate from that there should be a
pretty compelling reason.
And while I hear Claude's point that we may want to reserve
`/api/configsets/<name>` for some future metadata at some point...that doesn't
feel very compelling to me. "configset metadata" isn't a thing in Solr today.
And as Claude pointed out there are options (e.g. `Accept` header) for adding
that pretty seamlessly down the road.
--
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]
