Re: haproxy -dKcnv output

2023-05-31 Thread Tristan


Is it? In all the programming languages I use, the colon is followed by 
the return type (which for iif is str). 


my claim of mainstream-ness, was mainly meaning the ": in => out" order 
(one example would be most ML languages, Typescript, Java...) as opposed 
to ": out <= in" which I haven't seen being used so far


Of course the '=>' also implies 
return type and now I'm seeing two return types there.



[...]


I think it's pretty clear if you look at the usage in a config. Example 
from my production config:


     ssl_fc,iif(https,http)

That works like the pipeline operator in shell or various programming 
languages. Effectively the 'ssl_fc' result acts as the first parameter, 
it's just written a little differently.


I think we're just seeing it in a different way

I see converters (and shell pipe operations for that matter) as 
functions being chained, so their individual types must be those of a 
function (ie: in => out) and cannot be "str", since that is the type of 
a value


Running with this on the example of shell pipes, where the fact that 
things usually also support these forms:


tool [...args] file
tool [...args] -

is interpretable as a convenience parenthesis-less form of:
(tool [...args)(file)
(tool [...args])(-)

And the left-side portion of the pipe (not sure if that has a precise 
name?) is not an implicit first argument to the right side, but rather 
the only argument of a nameless function defined on the right side


In that view then

> var x = iif("foo","bar")
here x is a a function, of type bool => str

> var y = x(true)
and y is the result of giving a bool to a bool => str, and thus an str

> ssl_fc,iif(https,http)
is then pretty much akin to iif(https,http)(ssl_fc)

and that is why iif(str,str) being "bool => str" seems sensible to me

Now one can be pedantic and then argue to write it like this:
iif: (str,str) => (bool => str)

which is what many ML languages go with, and has some elegance, but is 
terrible for readability and that I wouldn't defend



     iif(str,str): bool => str

hides the type of the "important" parameter in the middle (and has the 
colon = return type problem).


That's a fair point admittedly; maybe I've just grown too used to that 
downside




     iif(str,str): str <= bool

at least has the parameter on an outer position, but the order is still 
reversed from the config.


fwiw while I said it seems to be an unusual notation order, it does have 
the benefit of being clear, indeed




     iif(bool,str,str): str

is reasonably similar to the config, but not identical


The similarity is specifically what I dislike about it, as it does seem 
a lot like this


http-request set-var(txn.proto) iif(ssl_fc,https,http)

should be a valid config line
or at least I think that wouldn't be entirely unreasonable to think that 
at first glance




     bool,iif(str,str): str

which I didn't mention before might possibly be the best choice?


I do quite like that

Best regards, and thanks for the bike-shedding exercise :D
Tristan



Re: haproxy -dKcnv output

2023-05-31 Thread Tim Düsterhus

Tristan,

On 5/31/23 12:28, Tristan wrote:

If fetches already have the output type after the colon, then the
converter should not have the input type after the colon, i.e.

      iif(str,str): bool => str

is confusing, because it looks like it returns a bool, ... I guess?


While this is mainly a feelings thing, I'd say that it is more
widespread (if not nowadays near-mainstream? not that this is
necessarily a good argument of course) to write it that way


Is it? In all the programming languages I use, the colon is followed by 
the return type (which for iif is str). Of course the '=>' also implies 
return type and now I'm seeing two return types there.



Another option might be listing the "implicit" parameter as a real
parameter:

      iif(bool,str,str): str


This one feels quite unnatural to me; unless you're familiar with the
underlying implementation of many OOP systems and see this as some kind
of extension method definition, the fact that the first parameter is
implicitly passed seems (to me) like it's coming out of nowhere


I think it's pretty clear if you look at the usage in a config. Example 
from my production config:


ssl_fc,iif(https,http)

That works like the pipeline operator in shell or various programming 
languages. Effectively the 'ssl_fc' result acts as the first parameter, 
it's just written a little differently. I also think it's the most 
important parameter, because it represents the actual data that the 
converter works with.


iif(str,str): bool => str

hides the type of the "important" parameter in the middle (and has the 
colon = return type problem).


iif(str,str): str <= bool

at least has the parameter on an outer position, but the order is still 
reversed from the config.


iif(bool,str,str): str

is reasonably similar to the config, but not identical

bool,iif(str,str): str

which I didn't mention before might possibly be the best choice?

Best regards
Tim Düsterhus



Re: haproxy -dKcnv output

2023-05-31 Thread Tristan
If fetches already have the output type after the colon, then the 
converter should not have the input type after the colon, i.e.


     iif(str,str): bool => str

is confusing, because it looks like it returns a bool, ... I guess?


While this is mainly a feelings thing, I'd say that it is more 
widespread (if not nowadays near-mainstream? not that this is 
necessarily a good argument of course) to write it that way




     iif(str,str): str <= bool



Though I guess this one is clear enough, if unusual (to me anyway)

Another option might be listing the "implicit" parameter as a real 
parameter:


     iif(bool,str,str): str


This one feels quite unnatural to me; unless you're familiar with the 
underlying implementation of many OOP systems and see this as some kind 
of extension method definition, the fact that the first parameter is 
implicitly passed seems (to me) like it's coming out of nowhere


Tristan



Re: haproxy -dKcnv output

2023-05-31 Thread Willy Tarreau
Hi all,

On Wed, May 31, 2023 at 10:02:45AM +0200, Tim Düsterhus wrote:
> Aurelien,
> 
> On 5/31/23 09:57, Aurelien DARRAGON wrote:
> > would not fit properly with existing representation for converters
> > within the doc
> > 
> > > iif(str,str): str <= bool
> > 
> > and
> > 
> > > iif(str,str): bool => str
> > 
> > could be good candidates (fetches are already represented using
> > "name(arg) : out"), although dconv would have to be patched anyway for
> 
> If fetches already have the output type after the colon, then the converter
> should not have the input type after the colon, i.e.
> 
> iif(str,str): bool => str
> 
> is confusing, because it looks like it returns a bool, ... I guess?
> 
> iif(str,str): str <= bool
> 
> is better, because the colon is followed by the output type, but the order
> of "reading" is a little funky with the actual input on the very right side.
> 
> Another option might be listing the "implicit" parameter as a real
> parameter:
> 
> iif(bool,str,str): str

Alright, at least it means we have something non-trivial to improve here,
so better not change it for 2.8 so that we have plenty of time to rethink
all this (including the way we present them in the doc) past the release.

Thank you, that's exactly what I needed ;-)
Willy



Re: haproxy -dKcnv output

2023-05-31 Thread Tim Düsterhus

Aurelien,

On 5/31/23 09:57, Aurelien DARRAGON wrote:

would not fit properly with existing representation for converters
within the doc


iif(str,str): str <= bool


and


iif(str,str): bool => str


could be good candidates (fetches are already represented using
"name(arg) : out"), although dconv would have to be patched anyway for


If fetches already have the output type after the colon, then the 
converter should not have the input type after the colon, i.e.


iif(str,str): bool => str

is confusing, because it looks like it returns a bool, ... I guess?

iif(str,str): str <= bool

is better, because the colon is followed by the output type, but the 
order of "reading" is a little funky with the actual input on the very 
right side.


Another option might be listing the "implicit" parameter as a real 
parameter:


iif(bool,str,str): str

Best regards
Tim Düsterhus



Re: haproxy -dKcnv output

2023-05-31 Thread Aurelien DARRAGON
> What I would find clear:
> 
> bool => iif(str,str) => str 


You're right Tim

But in the long term it could be great to share a common output format
with the doc as well (to find all relevant info from -dKcnv in the doc,
and vice versa)

While

> bool => iif(str,str) => str

and

> bool | iif(str,str) => str

would not fit properly with existing representation for converters
within the doc

> iif(str,str): str <= bool

and

> iif(str,str): bool => str

could be good candidates (fetches are already represented using
"name(arg) : out"), although dconv would have to be patched anyway for
this to work since it does not allow '=' nor '>' or '<' characters after
the ':' because of this regexp:

https://github.com/cbonte/haproxy-dconv/blame/4decf0b72eff0888195c2353ea149cd969110fe5/parser/keyword.py#L31



Re: haproxy -dKcnv output

2023-05-31 Thread Tim Düsterhus

Hi

On 5/30/23 22:09, Aurelien DARRAGON wrote:

$> haproxy -f test.conf -dKcnv | grep nbsrv
iif(string,string): str => bool



iif(string,string): bool => str




I don't rely on it, but frankly I find both variants confusing, because 
it does not follow the logical processing order.


What I would find clear:

bool => iif(str,str) => str

or

bool | iif(str,str) => str

or perhaps

iif(str,str): str <= bool

Best regards
Tim Düsterhus



Re: haproxy -dKcnv output

2023-05-30 Thread Willy Tarreau
On Tue, May 30, 2023 at 10:09:55PM +0200, Aurelien DARRAGON wrote:
> Dear haproxy users,
> 
> We recently noticed an inconsistency with haproxy -dKcnv output (which
> may be used to dump all available sample converters from the cli).
> 
> Here is how a converter is currently being represented in -dKcnv output:
> 
>   conv_name([arg1 ...]) : output_type => input_type
> 
> For instance with the iff converter this would look like this:
> 
> > $> haproxy -f test.conf -dKcnv | grep nbsrv
> > iif(string,string): str => bool
> 
> 
> According to the documentation, iff converter returns a string, either
> the first or the second argument, depending on whether the input value
> (boolean expected) is true or false.
> 
> Here is the catch: with the current output format, "=>" suggests a left
> to right transition. Thus it feels more logical to see input => output
> instead of output => input.
> 
> Input and output were probably mixed when the feature was first
> implemented and it was never updated since.
> 
> We would like to know your thoughts about this, whether you would be OK
> or not with us "inverting" output_type and input_type in -dKcnv output
> to remove this ambiguity for the 2.8 release:
> 
>   conv_name([arg1 ...]) : input_type => output_type
> 
> For the iff converter used as an example, this would result in the
> following output being produced:
> 
> > iif(string,string): bool => str
> 
> As '-dKcnv' support was added in 2.6, we would totally understand if you
> rely on the current output format and don't expect nor want it to be
> changed, but we would prefer to hear it from you if this is the case.

Thanks Aurélien for the explanation.

In short, I don't think anyone depends on it and I'm willing to merge
Aurélien's fix today before the release. However if we get a single
"please don't do that" before noon CEST today, I'll refrain from doing
it. It won't change in a stable version in any case, so better know
before releasing ;-)

Thanks,
Willy



Re: haproxy -dKcnv output

2023-05-30 Thread Aurelien DARRAGON
Pardon the few typos in the previous mail

> $> haproxy -f test.conf -dKcnv | grep iif
> iif(string,string): str => bool

Replace iff with iif :)

Regards,
Aurelien



haproxy -dKcnv output

2023-05-30 Thread Aurelien DARRAGON
Dear haproxy users,

We recently noticed an inconsistency with haproxy -dKcnv output (which
may be used to dump all available sample converters from the cli).

Here is how a converter is currently being represented in -dKcnv output:

  conv_name([arg1 ...]) : output_type => input_type

For instance with the iff converter this would look like this:

> $> haproxy -f test.conf -dKcnv | grep nbsrv
> iif(string,string): str => bool


According to the documentation, iff converter returns a string, either
the first or the second argument, depending on whether the input value
(boolean expected) is true or false.

Here is the catch: with the current output format, "=>" suggests a left
to right transition. Thus it feels more logical to see input => output
instead of output => input.

Input and output were probably mixed when the feature was first
implemented and it was never updated since.

We would like to know your thoughts about this, whether you would be OK
or not with us "inverting" output_type and input_type in -dKcnv output
to remove this ambiguity for the 2.8 release:

  conv_name([arg1 ...]) : input_type => output_type

For the iff converter used as an example, this would result in the
following output being produced:

> iif(string,string): bool => str

As '-dKcnv' support was added in 2.6, we would totally understand if you
rely on the current output format and don't expect nor want it to be
changed, but we would prefer to hear it from you if this is the case.


Thank you very much,

Aurelien