Bryan Boreham
Bryan Boreham
Closing as it's been in draft state for a long time. If you come back to it please re-open.
Hello from the bug-scrub! As a previous reviewer, the status remains that the code changes behaviour and the PR comments do not acknowledge this change, so I cannot accept the...
I observe it fails because `ToLower("ſſs")` is `"ſſs"`. However if we flip all the `ToLower`s to `ToUpper`, this case does work. Maybe we can use `unicode.SimpleFold`? (which I found used...
Hello from the bug-scrub! @sgrzemski do you think you will come back to this?
> so resources can be released, when Prometheus exits Can you clarify this please? What resources are retained past the exit of a process?
Always nice to say what you mean. Looking at the code changes, could we describe this PR as "Release PromQL engine resources on close, from tests" ? Calling `Close` from...
> `default(foo{}, default_val)` So `default(foo{}, 0) + default(bar{}, 0)` ? I don't think it decomposes as a function in PromQL's current AST - it needs to fill series labels which...
> Was `foo + default(0) bar` meant to be bidirectional? Yes - "add, defaulting to 0 when there is no value on the other side". I think you have a...
Nobody else likes `sum(foo, bar, baz)` ?
> Do you like it? It's essentially syntactic sugar for `sum({__name__=~"foo|bar|baz"})`, and you already said this is prone to the "vector cannot contain metrics with the same labelset" problem. Yes,...