ES|QL fix telemetry tests (usage stats) after promoting CATEGORIZE #117878
Conversation
luigidellaquila
added
>non-issue
auto-backport
elasticsearchmachine
added
v9.0.0
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
| @@ -163,4 +163,4 @@ setup: | |||
| - match: {esql.functions.cos: $functions_cos} | |||
| - gt: {esql.functions.to_long: $functions_to_long} | |||
| - match: {esql.functions.coalesce: $functions_coalesce} | |||
| - length: {esql.functions: 118} # check the "sister" test above for a likely update to the same esql.functions length check | |||
| - length: {esql.functions: 119} # check the "sister" test above for a likely update to the same esql.functions length check | |||
There was a problem hiding this comment.
Shouldn't the other test be changed as well?
Also, I am not clear about the "promotion" of categorize. Looking at EsqlCapabilities I see it's still a snapshot only feature test-wise.
There was a problem hiding this comment.
Also, if this is about snapshot/non-snapshot thingies, shouldn't the PR run with test-release label?
Even the issue it's trying to fix (#117862) comes from test-release CI runs.
There was a problem hiding this comment.
I see it's still a snapshot only feature test-wise.
It's going to be un-snapshotted soon!
Thanks for pointing out the test-release label, I was unaware of that one.
There was a problem hiding this comment.
The other test is good because the function existed already (but only in snapshot).
The promotion happened in EsqlFunctionRegistry, I think the capabilities should be changed as well. Since we are on it, let me do it now
There was a problem hiding this comment.
Yeah, the un-snapshotting of the capability in #117881 should make sure that the CATEGORIZE test also run on release tests. Thanks for looking into this Luigi!
|
Update: Actually, let me double check that. I think the change to the Update 2: Yes, that's the case. This confirms the PR is doing the right thing! |
There was a problem hiding this comment.
LGTM if tests pass.
Let's be reasonable about test-release label: we are doing our best to prove our own changes pass this label's test, if there are other issues unrelated to ES|QL with test-release, ideally and if we have time we can create GH issue for the appropriate teams, otherwise it's ok to merge imo.
💔 Backport failed
You can use sqren/backport to manually backport by running |
|
Backport #117915 |
After promoting CATEGORIZE function to non-snapshot, release tests started failing because we did not account for that function in the usage stats.
Fixes: #117862
Fixes: #117905
Fixes: #117907
Fixes: #117908