lucidworks / zeppelin-solr

Apache Solr interpreter for Apache Zeppelin
Apache License 2.0
28 stars 4 forks source link

Remove client-close logic in StreamingResultsIterator #33

Closed gerlowskija closed 3 years ago

gerlowskija commented 3 years ago

StreamingResultsIterator had code that conditionally closed HttpSolrClient's in its hasNext() method. This code originally was copied from a different project, where this decision made a little more sense. But in zeppelin-solr, SolrClients are always tied to the lifecycle of the interpreter and should never be closed prematurely.

This PR removes the errant closure, which was causing any request after a zero-result query to error out.

gerlowskija commented 3 years ago

I'm going to merge this change as is since it fixes a clear problem.

That said we may want to circle back and do a broader evaluation of our lifecycle strategy here for these HttpSolrClient's. Is there a reason to share clients across requests/"notes" at all? We wouldn't have to worry about lifecycle bugs or connection pool errors, etc if these clients were created lazily in a try-with-resources right as they're needed.