> We can only hypothesize _why_ a client is dependent in the first place ...
> Perhaps to tweak/tune advanced options, timeouts. Perhaps to instrument mTLS
> details
Another use-case to add to this list would be auth settings. I'm
struggling to come up with a concrete example this minute, but I know
I've written SolrJ code that customized the underlying HttpClient for
auth-related purposes.
> "CloudSolrClient" is an intuitive/obvious name to a user that wants to talk
> to SolrCloud [...] HTTP protocol or wether the client is using whatever HTTP
> library is all an implementation detail.
+1 I like the idea of keeping implementation details out of the name
of any types we're putting front-and-center for SolrJ users. But I
share Jan's concern about breaking clients who rely on a particular
underlying client type.
My favorite idea so far is probably Jan's point that balancing those
two gets a lot easier if we introduce some "new" name like
"ClusterSolrClient" as the long term successor to
CloudSolrClient/BaseCloudSolrClient. It'd be nice to keep the name
'CloudSolrClient' itself for the sake of continuity, but
ClusterSolrClient at least preserves the reasons we like
'CloudSolrClient' as a name and it makes keeping backcompat pretty
easy:
I guess concretely, this would look something like:
1. Create a new class, 'ClusterSolrClient', that's a trivial extension
of BaseCloudSolrClient. (i.e. `class ClusterSolrClient extends
BaseCloudSolrClient {}`)
2. Add a builder for the new 'ClusterSolrClient' that can create
either the apache or jetty-powered CloudSolrClient based on the
builder methods invoked.
3. Deprecate BaseCloudSolrClient, CloudSolrClient, CSC.Builder, and
CloudHttp2SolrClient.Builder for 9.0, directing users over to the new
ClusterSolrClient and its builder.
4. Remove the deprecated classes in 10.0
Does something like this sound do-able?
Jason
On Wed, Mar 16, 2022 at 10:50 AM Mike Drob <md...@mdrob.com> wrote:
>
> I feel like CloudSolrClient doesn't imply anything about HTTP 1 or 2,
> anything about Apache or Jetty (or java.net.http). If we have exposed those
> internal details in some ways, then that is unfortunate and should be
> addressed.
>
> I personally never use CloudHttp2SolrClient because I kind of assumed that it
> was an implementation detail and the various builders would give me the http2
> client when I needed it. Maybe that's not the case. I've never thought about
> it too much. CloudSolrClient looks like the "simpler" one to use so that's
> what people gravitate towards.
>
> A quick look in my editor suggests that we have 100 uses of CloudSolrClient,
> including some in the ref guide. If we want to deprecate this, then we should
> update our documentation to guide people away from it as well. I suspect that
> if we try to examine which uses of CloudSolrClient in our code could just be
> SolrClient, we wouldn't make much progress on this though.
>
> I know this isn't offering much in the way of solutions, but I'm mostly
> trying to say that I agree it is a problem.
>
>
> Mike
>
> On Wed, Mar 16, 2022 at 12:05 AM David Smiley <dsmi...@apache.org> wrote:
>>
>> On Tue, Mar 15, 2022 at 8:47 AM Jan Høydahl <jan....@cominvent.com> wrote:
>>>
>>> I re-opened SOLR-15223 to highlight that we are still blocked by this
>>> decision.
>>>
>>> I don't clearly see the full effects of your suggestion right now. Does
>>> your proposal also involve deprecating CloudHttp2SolrClient as a separate
>>> class?
>>
>>
>> No; it would stay. Perhaps ideally it would have a name reflecting it uses
>> the Jetty client but no big deal; it can stay as-is. Its name already isn't
>> necessarily true; you can use this class (and thus the Jetty client) and
>> tell it not to use Http2 :-). I'm reminded that HdfsDirectory doesn't
>> require HDFS :-). (It requires the HDFS client libs but not necessarily an
>> HDFS backend, if you're curious).
>>
>>>
>>> I would imagine users with existing SolrJ code would after upgrading get an
>>> instance of BaseCloudSolrClient (with a new name) using Jetty client under
>>> the hood? What if that application code assumes org.apache.http as client
>>> and tries to obtain HttpSolrClient and even org.apache.http objects based
>>> on CloudSolrClient? The code would fail since the contract is broken.
>>
>>
>> If the client/user truly assumes org.apache.http well clearly they will be
>> disrupted by this change. You want to call that a "contract" -- shrug; I
>> call it an implementation detail that can change :-). They may be calling
>> getLbClient and may be using the LBHttpSolrClient subclass of LBSolrClient,
>> perhaps. Or similarly specifying builder options relating to this advanced
>> option. It's possible and it's undeniable _some_ clients will be impacted.
>> We can only hypothesize _why_ a client is dependent in the first place (vs.
>> perhaps an accidental dependency/assumption e.g. in dependency management).
>> Perhaps to tweak/tune advanced options, timeouts. Perhaps to instrument
>> mTLS details; although I know from experience it can be done without calling
>> special methods on builders; it can be done via setting special system
>> properties referring to one's own classes that are called in certain ways.
>> If you do that (and we have), the way to do it for the Apache based client
>> differs from Jetty; we've done it for both because Solr uses both
>> internally. Anyway, this is off the beaten path of most users.
>>
>>>
>>>
>>> With the current pure deprecation and switch to CloudHttp2SolrClient,
>>> existing users' code would continue to work..
>>
>>
>> Hey, this is a major release; let's not hold ourselves to a standard that is
>> too onerous for us to maintain. We can make our intentions clear in upgrade
>> notes.
>>
>> ~ David
>>
>>>
>>> Jan
>>>
>>>
>>> 14. mar. 2022 kl. 15:40 skrev David Smiley <dsmi...@apache.org>:
>>>
>>> I want to bring an important SolrJ decision to the dev list.
>>>
>>> There's a JIRA issue https://issues.apache.org/jira/browse/SOLR-15223
>>> "Deprecate HttpSolrClient and friends in 9.0"
>>>
>>> Sounds great by the title -- we want to transition over time to the Jetty
>>> client instead. Jan submitted a PR to deprecate CloudSolrClient and some
>>> others, and I approved it because these classes intimately assume the
>>> Apache HttpClient. It's merged.
>>>
>>> But I have serious doubts now and wish to discuss it with the dev list.
>>> Copying my last message on the issue:
>>>
>>>> Now that I'm "seeing" the results of this in my IDE, seeing the
>>>> cross-through of deprecated usage on innocent looking classes like
>>>> CloudSolrClient in particular, I have doubts on the approach.
>>>> "CloudSolrClient" is an intuitive/obvious name to a user that wants to
>>>> talk to SolrCloud. The particulars of which HTTP protocol or wether the
>>>> client is using whatever HTTP library is all an implementation detail.
>>>> Ideally such decisions would be done in the builder, either a common
>>>> builder or if not then a builder specific to those libraries if needed
>>>> (less nice but acceptable IMO).
>>>>
>>>> The easiest way to get there is to rename CloudSolrClient to
>>>> CloudHttp1SolrClient in one commit (merge it) and then rename
>>>> BaseCloudSolrClient to simply CloudSolrClient in the next. Then add a
>>>> Builder to this class that is the one in Http2; subclass it or something
>>>> (details TBD).
>>>>
>>>> WDYT?
>>>>
>>>> Of course, today they are separated by their classes. Maybe we should
>>>> simply convey the deprecation intent in the upgrade notes as an advanced
>>>> warning, but not deprecate CloudSolrClient in particular.
>>>
>>>
>>> Jan replied:
>>>
>>>> Since we did not deprecate these in 8.x, we still have a back-compat
>>>> promise to keep these classes around in 9.x, and thus also the old http
>>>> client. But perhaps we are breaking that promise already in SOLR-16061, so
>>>> maybe we can change even more
>>>>
>>>> I don't like the CloudHttp2SolrClient naming either, would prefer the Http
>>>> client to be abstracted away so that it could be swapped out as an impl
>>>> detail, but it was not designed that way. I fear that re-using the same
>>>> class name but with slightly different contract is harder to explain than
>>>> a clear deprecation message in the IDE pointing you to the replacement.
>>>>
>>>> Perhaps the one client to rule them all in 10.0 should be
>>>> ClusterSolrClient? And aim for it to be constructed with either a Jetty
>>>> client or JDK8-HttpClient as backbone through different factories/builder?
>>>
>>>
>>> How is the contract between CloudSolrClient & BaseCloudSolrClient different
>>> Jan? I suspect if there's breakage, it'd be relatively obscure options on
>>> the builder.
>>>
>>> ~ David Smiley
>>> Apache Lucene/Solr Search Developer
>>> http://www.linkedin.com/in/davidwsmiley
>>>
>>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@solr.apache.org
For additional commands, e-mail: dev-h...@solr.apache.org
