dsmiley commented on PR #3233: URL: https://github.com/apache/solr/pull/3233#issuecomment-2715838346
Thanks for your feedback Alex! > 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). I *think* this will be less of an issue now because the HttpClient (thus everything attached to it) goes along for the ride. This PR shows that we no longer need to copy them when the httpClient is provided in the builder. But I suppose that if a user creates an HttpClient on their own for whatever reason, then the onus is on them to configure it with whatever listeners are desired; previously this was separate. But that's true with all the other settings in the builder associated with the HttpClient construction (e.g. everything createHttpClient looks at in the builder). It underscores a point that if you provide an HttpClient to the builder, it's an expert use-case; you know what you are doing because we don't document every last setting, wether it applies or not if someone provides the client. > 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?). I've only seen one switch in the project's history (Apache -> Jetty) so I don't think it's worth insulating the differences. New abstractions are not free; abstractions should be documented as well; don't benefit from knowledge the user might already have in Jetty and they can get in the way. I'd rather see Solr reduce code wherever it can; removing frivolous abstractions is one. Solr's listener factory thing being removed here doesn't apply to the HttpJdkSolrClient (or else you'd have seen the impact in your review). -- 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
