gerlowskija commented on PR #2470: URL: https://github.com/apache/solr/pull/2470#issuecomment-2121513823
Thanks for the quick review Eric! > I tagged some places where it would be nice ot have more descriptive variable names, however I suspect you are trying to minimize change Your suspicion is correct. This PR aims to convert the API(s) to JAX-RS without delving too deeply into cleaning up the underlying implementation. I think some of the stuff you commented on is innocuous enough that I can be tempted into making a few cleanups though, particularly some of the naming and javadoc/comment issues. > Do we still need DistribFileStore in a world with ClusterFileStore? Yes, we do. There's some naming-related confusion going on here that I've definitely caused: 1. `FileStore` is an interface that defines the operations that Solr's file-store has to support. 2. `DistribFileStore` is the lone implementation of the `FileStore` interface. 3. `ClusterFileStore` is the entrypoint for the relevant v2 APIs. `ClusterFileStore` is, on its surface, a pretty bad naming choice given the pre-existing `FileStore` interface. It 100% appears to be an implementation of that interface, but it's not. I do have an excuse, though it may be a bad one 😛 I chose `ClusterFileStore` for the v2 API class name to be consistent with a naming convention we've had since creating the "api" gradle module earlier this year. (i.e. interfaces in "api" module are `FooApi`, with classes in solr-core being `Foo`). The filestore (for reasons I still don't really understand) makes APIs available under two paths: `/api/cluster/files` and `/api/node/files`. So I chose `ClusterFileStoreApi`/`ClusterFileStore` partially in an attempt to distinguish between these two sets of filestore APIs, and partially to maintain consistency with the existing naming convention. The end result is clearly confusing; very open if anyone has other suggestions that'd avoid the ambiguity and confusion!? -- 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]
