aparnasuresh85 commented on PR #2392: URL: https://github.com/apache/solr/pull/2392#issuecomment-2041500688
> > +1 to the change. needs "tidy" clearly (you don't have a space to the left of the parenthesis) > > Also something was eating at it me a bit... the method you added, `resolveDocCollection`, returns a DocCollection but in HttpSolrCall.init you added a call to it that does nothing with the response. Feels weird. Instead, shouldn't HttpSolrCall.getCoreByCollection use this method? > > I agree. The reason I kept the call to resolveDocCollection() outside of getCoreByCollection() is because the doc around the latter (if getCoreByCollection returned null ) said "// this collection exists , but this node does not have a replica for that collection". > > Instead, should we throw a SolrException (collection or alias not found) if inside getCoreByCollection, the call to resolveDocCollection() returned null? Maybe not. I see a number of tests fail if an exception is thrown in case collection is null from resolveDocCollection(). The tests pass otherwise. I will send a new PR. -- 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]
