Remove duplicate dependency declaration for http client

[danielmitterdorfer]

We disable transitive dependencies in our build plugin
for all dependencies except for the group org.elasticsearch`.
However, in the reindex plugin we depend on the REST client
and declare its dependencies again which is not necessary
(and led to problems with conflicting versions in #19281.
After a fair amount of digging / debugging the build I am
still not sure why though.)

With this PR we remove the duplicate declaration.

[nik9000]

LGTM. @javanna, I think it is important you have a look too.

I didn't know that that is how we handled Elasticsearch's transitive dependencies.

[javanna]

this change looks good to me. I am not sure whether long term we should rather disable transitive deps for o.e.client. maybe @rjernst can comment on that?

Note that the conflict problem originates from httpaasyncclient which depends on httpclient 4.5.2 and httpcore 4.4.5, but httpclient 4.5.2 depends on httpcore 4.4.4. Weird situation.

We should also figure out whether there may be problems with commons-codec and commons-logging versions, I think Daniel found problems there too.

[nik9000]

We should also figure out whether there may be problems with commons-codec and commons-logging versions, I think Daniel found problems there too.

So long as we're just removing duplicate deps its ok with me. If we start getting diverging version numbers I think we're in a bad place.

[rjernst]

I am not sure whether long term we should rather disable transitive deps for o.e.client. maybe @rjernst can comment on that?

No, I don't think we should. We should not pull in anything in o.e.client that is not actually needed by the client.

[nik9000]

No, I don't think we should. We should not pull in anything in o.e.client that is not actually needed by the client.

I don't understand your answer. If we depend on o.e.client like we do in reindex should we have to list its deps in reindex or not? We don't now.

I don't think we're talking about allowing o.e.client to skip declaring its own transitive dependencies.

[rjernst]

If we depend on o.e.client like we do in reindex should we have to list its deps in reindex or not? We don't now

No, I don't think we should, for the same reason we do not for any plugin/module that depends on core. One of the main reasons for disabling transitive dependencies is to ensure we vet all dependencies. We have done that for o.e.client, so why would we need to do it again inside reindex?

[javanna]

thanks for the explanation Ryan, makes sense. LGTM then

[danielmitterdorfer]

@nik9000, @javanna, @rjernst thanks for the review.