Allow VertxHttpClientBuilder to be seeded with a base WebClientOptions object, allowing more customization of the Vertx HTTP Client

[capistrant]

Description

fixes #7252

Allows user to seed the Vert.x Http Client Builder with a custom WebOptions object if they want to modify configs of this object. As the issue specs out, this is not the most ideal solution, I think using additionalConfig like the other HttpClient implementations do would be best. But that seems like a breaking change since it would be more invasive.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change
• Chore (non-breaking change which doesn't affect codebase;
  test, version modification, documentation, etc.)

Checklist

• Code contributed by me aligns with current project license: Apache 2.0
• I Added CHANGELOG entry regarding this change
• I have implemented unit tests to cover my changes
• I have added/updated the javadocs and other documentation accordingly
• No new bugs, code smells, etc. in SonarCloud report
• I tested my code in Kubernetes
• I tested my code in OpenShift

[shawkins]

Looks good to me. Can you ready to take it out of draft?

[capistrant]

Looks good to me. Can you ready to take it out of draft?

did that just now. Was working on running it a bit in local k8s env and noodling on how we could actually test that the configs are getting applied. Especially in the sense of not having it regress in future and quietly stop taking effect. Running in dev, I did a little bit on Friday and will do some more today. The testing strategy, I'm still stumped on

[shawkins]

did that just now. Was working on running it a bit in local k8s env and noodling on how we could actually test that the configs are getting applied. Especially in the sense of not having it regress in future and quietly stop taking effect.

Actually I see there is something we originally missed. The VertxHttpClientFactory.additionalConfig method should be called by VertxHttpClientBuilder.build method. That is consistent with the fabric8 factories and somewhat redundant with the ability to pass in a WebClientOptions. If you wire in that method, you should be able to check that your passed in options is in use.

I'm not opposed to having both ways of handling the WebClientOptions as subclassing is probably less obvious than looking for a builder method.

cc @manusa

[capistrant]

did that just now. Was working on running it a bit in local k8s env and noodling on how we could actually test that the configs are getting applied. Especially in the sense of not having it regress in future and quietly stop taking effect.

Actually I see there is something we originally missed. The VertxHttpClientFactory.additionalConfig method should be called by VertxHttpClientBuilder.build method. That is consistent with the fabric8 factories and somewhat redundant with the ability to pass in a WebClientOptions. If you wire in that method, you should be able to check that your passed in options is in use.

I'm not opposed to having both ways of handling the WebClientOptions as subclassing is probably less obvious than looking for a builder method.

cc @manusa

#7252 did talk about this as an alternative I considered. The reason I did not propose it was to avoid proposing what would be a breaking change for folks not using the default factory. That is at least how I interpreted the change, that it could be breaking. If that is ok, I could pursue that instead. It does make the most sense in terms of keeping patterns consistent across the project

[capistrant]

did that just now. Was working on running it a bit in local k8s env and noodling on how we could actually test that the configs are getting applied. Especially in the sense of not having it regress in future and quietly stop taking effect.

Actually I see there is something we originally missed. The VertxHttpClientFactory.additionalConfig method should be called by VertxHttpClientBuilder.build method. That is consistent with the fabric8 factories and somewhat redundant with the ability to pass in a WebClientOptions. If you wire in that method, you should be able to check that your passed in options is in use.

I'm not opposed to having both ways of handling the WebClientOptions as subclassing is probably less obvious than looking for a builder method.

cc @manusa

@shawkins I opened up #7384 as an alternative to this PR, which uses the additionalConfig method already available. As you'll see the generic factory in the builder ties our hands there, but maybe we can at least provide the functionality to folks using the stock factory. Appreciate if you can take a look and offer thoughts!

[shawkins]

@capistrant thank you for the additional pr. Let's see if @manusa agrees with that one, then we can probably close this one.