gerlowskija commented on code in PR #1704: URL: https://github.com/apache/solr/pull/1704#discussion_r1233125232
########## solr/core/src/java/org/apache/solr/handler/admin/api/CoreReplicationAPI.java: ########## @@ -46,12 +50,24 @@ public CoreReplicationAPI(SolrCore solrCore, SolrQueryRequest req, SolrQueryResp @GET @Path("/indexversion") - @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2}) + @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML, BINARY_CONTENT_TYPE_V2}) @PermissionName(CORE_READ_PERM) public IndexVersionResponse fetchIndexVersion() throws IOException { return doFetchIndexVersion(); } + @GET + @Path("/files/generation/{gen}") Review Comment: [Q] For a lot of these v2 APIs, I've been working from a [spreadsheet](https://docs.google.com/spreadsheets/d/1HAoBBFPpSiT8mJmgNZKkZAPwfCfPvlc08m5jz3fQBpA) that describes what the new endpoints should be. In the spreadsheet, the suggested v2 API was just `GET /cores/coreName/replication/files`. (I think at the time I was assuming "generation" would be a required query parameter, rather than a path-param. Though it's possible I didn't even know about it at that point.) Not that we have to stick to that necessarily - if you think it makes sense to keep "generation" in the path, it's def an option. Any thoughts? ########## solr/solr-ref-guide/modules/deployment-guide/pages/user-managed-index-replication.adoc: ########## @@ -433,9 +433,27 @@ http://_follower_host:port_/solr/_core_name_/replication?command=details `filelist`:: Retrieve a list of Lucene files present in the specified host's index. + + +==== +[.tab-label]*V1 API* + [source,bash] +---- http://_host:port_/solr/_core_name_/replication?command=filelist&generation=<_generation-number_> + +---- +==== ++ Review Comment: [+1] Thanks for thinking of the docs! ########## solr/core/src/java/org/apache/solr/handler/admin/api/ReplicationAPIBase.java: ########## @@ -38,10 +68,140 @@ public ReplicationAPIBase( } protected CoreReplicationAPI.IndexVersionResponse doFetchIndexVersion() throws IOException { + ReplicationHandler replicationHandler = + (ReplicationHandler) solrCore.getRequestHandler(ReplicationHandler.PATH); + return replicationHandler.getIndexVersionResponse(); + } + protected NamedList<Object> doFetchFiles(long generation) { ReplicationHandler replicationHandler = (ReplicationHandler) solrCore.getRequestHandler(ReplicationHandler.PATH); + return getFileList(generation, replicationHandler); + } - return replicationHandler.getIndexVersionResponse(); + protected NamedList<Object> getFileList(long generation, ReplicationHandler replicationHandler) { + final IndexDeletionPolicyWrapper delPol = solrCore.getDeletionPolicy(); + final NamedList<Object> filesResponse = new NamedList<>(); + + IndexCommit commit = null; + try { + if (generation == -1) { Review Comment: [Q] Mostly thinking out loud here, but I wonder whether we should just fall back on "-1" as a default. (Right now we require users to provide an explicit generation value). Probably not something to handle in this PR regardless, just wanted to jot down the thought somewhere... ########## solr/core/src/java/org/apache/solr/handler/admin/api/CoreReplicationAPI.java: ########## @@ -46,12 +50,24 @@ public CoreReplicationAPI(SolrCore solrCore, SolrQueryRequest req, SolrQueryResp @GET @Path("/indexversion") - @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2}) + @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML, BINARY_CONTENT_TYPE_V2}) @PermissionName(CORE_READ_PERM) public IndexVersionResponse fetchIndexVersion() throws IOException { return doFetchIndexVersion(); } + @GET + @Path("/files/generation/{gen}") + @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML, BINARY_CONTENT_TYPE_V2}) + @PermissionName(CORE_READ_PERM) + public NamedList<Object> fetchFiles( Review Comment: [0] The method name "fetchFiles" sounds a little bit like you're getting the file-content itself, rather than a list of file names/metadata. Might be worth updating. [-0] One of the values of the JAX-RS framework is that we can be explicit about the inputs and outputs of our APIs. If possible, we should avoid returning Namedlist here and instead return some other strongly typed class that makes it clearer what the response should look like? -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org