microbean-helm icon indicating copy to clipboard operation
microbean-helm copied to clipboard

Abstract ManagedChannel Creation

Open lewisheadden opened this issue 6 years ago • 5 comments

While using this project, I found myself wanting to customize the ManagedChannel that was created. While I saw there was an option to override buildChannel, I thought it might be nicer to provide that as a dependency to Tiller instead.

As such, I created a ManagedChannelFactory interface which is used to create a ManagedChannel from a LocalPortForward. I also provide the default implementation (DefaultManagedChannelFactory) that migrates existing logic from the current buildChannel method. It also allows the provision of a ManagedChannelConfigurer which is passed the ManagedChannelBuilder and is able to manipulate it before it is finally built.

This allows users to do the following if they simply want to configure settings on the ManagedChannel instead of having to subclass Tiller.

final Tiller tiller = new Tiller(client, new DefaultManagedChannelFactory((builder) -> {
    builder
        .keepAliveTime(90L, TimeUnit.SECONDS)
        .keepAliveTimeout(30L, TimeUnit.SECONDS);
}));

Let me know if I've not explained anything well, or if you have any concerns or changes you'd like.

lewisheadden avatar Feb 01 '19 19:02 lewisheadden

Hello; thanks for using microBean Helm.

I like the general idea here; since what you want to do is not prohibited currently, I'd like to let this sit for a bit so I can chew on it.

This project is slightly unique in that one of my design goals/constraints is to expose as tiny an API surface area as possible, as it's just me on nights and weekends. Perhaps what you want to do is best served by simply accepting something like a Supplier<ManagedChannel>, and we just leave it at that.

ljnelson avatar Feb 04 '19 23:02 ljnelson

@ljnelson Totally understood! I was trying to keep it to one component but the dependency wiring into Tiller seemed a little weird.

Take your time and let me know what you decide! Thanks for your hard work!

lewisheadden avatar Feb 05 '19 03:02 lewisheadden

I'm going to start by adding the ability to take in a Function<? super LocalPortForward, ? extends ManagedChannel> at construction time. After that, we can explore (through concrete usage, I hope) how painful it is to supply such a Function, and introduce, perhaps, additional implementations of it. Watch this space!

ljnelson avatar Feb 14 '19 00:02 ljnelson

@lewisheadden I have deployed microbean-helm version 2.12.3.0.0.1-SNAPSHOT to the Sonatype snapshots repository. I would appreciate any testing you might be able to perform.

ljnelson avatar Feb 14 '19 00:02 ljnelson

@ljnelson I recently left the job that this PR was related to and am taking some time off between roles but @jsvk is still working on this and might be able to give you some more timely insight.

lewisheadden avatar Feb 25 '19 16:02 lewisheadden