stillalex commented on PR #3233: URL: https://github.com/apache/solr/pull/3233#issuecomment-2714527929
interesting change, I think it looks good and the new version of the code looks much cleaner, +1 from me. just 2 thoughts (feel free to ignore if not relevant): - there are some places where we create new clients based on existing clients settings, so would we need to make sure the listeners are being copied over? (not sure if this is a problem here, I remember running into this pattern a while back). - removing a solr abstraction in favor of a jetty abstraction brings the code closer to jetty clients by creating this hard dependency. and while I have not been around enought to tell, it feels like long term it would be better to depend less on specific http clients, giving the project some freedom in picking a new impl if needed (would this work with the new jdk http client?). -- 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
