Skip to content

Conversation

capistrant
Copy link

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

@capistrant capistrant marked this pull request as draft September 27, 2025 02:57
Copy link
Contributor

@shawkins shawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@capistrant capistrant marked this pull request as ready for review September 29, 2025 15:04
@capistrant
Copy link
Author

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
Copy link
Contributor

shawkins commented Oct 3, 2025

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vert.x HttpClient should provide ability to customize WebClientOptions

2 participants