Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Armeria-based HttpClient #5307

Closed
ikhoon opened this issue Jul 5, 2023 · 9 comments
Closed

Add Armeria-based HttpClient #5307

ikhoon opened this issue Jul 5, 2023 · 9 comments

Comments

@ikhoon
Copy link
Contributor

ikhoon commented Jul 5, 2023

Is your enhancement related to a problem? Please describe

In Armeria, I'm implementing client-side load balancing for the service discovery of Kubernetes clusters. As Armeria already provides its own powerful HTTP client built on top of Netty that supports both HTTP/1 and HTTP/2, it is weird to use other network engines that Kubernetes client provides such as JDK HttpClient and OkHttp.

Describe the solution you'd like

If the maintainers are open to accepting a PR for Armeria integration, I'd like to send a pull request. It would be also useful for other users to have the new option of a new backend.

Describe alternatives you've considered

No response

Additional context

No response

@manusa
Copy link
Member

manusa commented Jul 5, 2023

If the maintainers are open to accepting a PR for Armeria integration, I'd like to send a pull request. It would be also useful for other users to have the new option of a new backend.

Since plugging in your own HttpClient implementation is trivial now, there are two options to address this.

  • a) Host the Armeria Fabric8 HttpClient implementation on the Armeria repository
  • b) Host the Armeria Fabric8 HttpClient implementation on the Fabric8 Kubernetes Client repository

From my perspective/IMHO, b makes sense only if this Armeria HTTP client is something that's used in other projects too, then it makes sense to host it and maintain it here. If not, then maybe a might make more sense, since in the end it will be something specific to that project and that won't be reusable elsewhere.

@ikhoon
Copy link
Contributor Author

ikhoon commented Jul 13, 2023

I also prefer b option since the Armeria client is a general HTTP/1 and HTTP/2 client runtime which other Fabric8 users might be interested in. Other OSS projects provide an integration layer for Armeria such as OpenTelemery and Http4s and so on.

Armeria team is currently implementing WebSocket client which is necessary for Fabric8 HttpClient implementation. If it is merged and released, I will send a PR to Fabric8 Kubernetes Client repository.

@andreaTP
Copy link
Member

andreaTP commented Jul 13, 2023

@ikhoon I'm in favor of starting with option a for various reasons:

  • speed of development: contributing code to a repo that you own should be much faster
  • maintenance: in the fabric8 team there is about 0 knowledge of the Armeria client and, answering issues or fixing bugs is going to be troublesome for us
  • release cycles: in a separate repo you can fix bugs and release new versions at your own peace

I think that an ideal path forward (at least on our side) would be:

  1. you start the HTTP client in your repo
  2. you release it to the community and start to receive feedback
  3. as soon as you have reached a minimum stability we re-evaluate integrating in this repo

wdyt?

@ikhoon
Copy link
Contributor Author

ikhoon commented Aug 25, 2023

Understood.

Armeria team is currently implementing WebSocket client which is necessary for Fabric8 HttpClient implementation.

WebSocket client has been added to Armeria. Let me implement Fabric8 HttpClient implementation on the Armeria repository first.

@ikhoon ikhoon closed this as completed Aug 25, 2023
@manusa
Copy link
Member

manusa commented Aug 25, 2023

Let me implement Fabric8 HttpClient implementation on the Armeria repository first.

Sounds good. Please, if possible, reference this issue in the PR you create in the Aremria's repo so that we can keep track of it.

@ikhoon
Copy link
Contributor Author

ikhoon commented Sep 4, 2023

While testing Armeria backend for Fabic8 HttpClient with MockWebServer, the tests failed due to the mock server does not support OPTIONS method which is used to attempt to upgrade HTTP/2.
fabric8io/mockwebserver#79 It would be very helpful to merge the PR and release a new version to handle the blocker. 🙇‍♂️

@manusa
Copy link
Member

manusa commented Sep 5, 2023

There are multiple improvements for the MockWebServer pending.
This week we'll discuss if we move the module to this repo.
In any case, consider the PR content to be merged and available in a couple of days.

@manusa
Copy link
Member

manusa commented Sep 25, 2023

Hi @ikhoon,

Your changes in fabric8io/mockwebserver#79 will be available as part of 6.9-SNAPSHOT which will be made available in the next SNAPSHOT publish run.

@ikhoon
Copy link
Contributor Author

ikhoon commented Sep 26, 2023

Thanks. ❤️ I will download the version in my PR.

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

No branches or pull requests

3 participants