Hi Michal,

On Mon, Jan 09, 2017 at 02:00:19PM +0100, Micha?? wrote:
> Hello!
> It's my first PR to haproxy, so please tell me if anything still wrong.
> I've read CONTRIBUTING.
> 
> This patches implements possiblity to define different host (agent-host) for
> agent checks in config and they also allow changing agent-host and
> agent-send
> variables via CLI/socket. We wonna use this options to dynamically swap
> backends
> without reloading haproxy. agent-host will allow us to control weight and
> status
> of servers from single place, so we can enable server when it's ready
> (warmed)
> instead just after healthcheck passing.

I remember a recent discussion on this subject, I don't remember if it was
here on the list or with Baptiste or someone else, but that's definitely
welcome!

> I think this change isn't touching any
> hot paths and will find adopters,

I think it will be useful to make it easier to control server statuses from
a central place.

> because reloading haproxy with all SSL stuff
> and losing statstics for less dynamic backends is pain.
> 
> In my opinion changes in code don't require any comments. I documented those
> features of course, so everyone can use them.

Indeed, however a short description of the changes in the commit messages 
would help. Please keep in mind that merged patches are review again later :
  - when backporting fixes and the rare few minor improvements
  - when bisecting to find the cause of a bug.

In this case it's really important that all the useful information is
visible in the git log to help take the appropriate decision (eg: backport
yes/no, or this is totally unrelated to my problem).

I'd personally prefer if the "agent-host" was renamed to "agent-addr" to
be more consistent with the "addr" parameter we already have for the
checks and which is used at a few places on the CLI. Also in the HTTP
world, "host" is a bit connoted as the string found in the header field
carrying the same name :-)

Otherwise your changes look fine, I'm willing to merge them, I don't
see any risk there.

Thanks!
Willy

Reply via email to