dsmiley commented on pull request #2605:
URL: https://github.com/apache/lucene-solr/pull/2605#issuecomment-962233248


   Please don't force-push PRs as it inhibits a reviewers ability to see what 
was changed.
   
   This time I ran via the CLI instead of IntelliJ and it passed.  Maybe my 
IntelliJ dependency versions were stale (Caffeine was updated recently) so I 
re-generated and still same result with IntelliJ.  Weird; is it just me?
   
   I constrained the try-catch around just the call to `toTermSet = 
toSearcher.getDocSet(toDeState);` and it passed; I don't get your point Thomas 
about resultList.
   
   The test adds tons of over 3k docs to Solr one-by-one instead of batching.  
Not a deal breaker but sad when I see it.  Try this:
   ```
       SolrClient client = new EmbeddedSolrServer(h.getCore());
       client.add(persons);
   ```
   
   But hold up...
   
   This logic being added, I noticed, is able to see the filter cache config 
without SolrCache having isAsync() and therefore I think we can make this thing 
reproduce every time without actually adding any data for a test.  See my 
related comment in JIRA.  Wouldn't that be better?  Better to eagerly fail than 
to have a failure sneak up on you in production.


-- 
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]

Reply via email to