feat!: add function sync script with upstream#138
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #138 +/- ##
==========================================
- Coverage 98.93% 98.54% -0.39%
==========================================
Files 15 15
Lines 6560 6959 +399
==========================================
+ Hits 6490 6858 +368
- Misses 70 101 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@waynexia do you think we can remove |
Maybe we should keep it for compatibility with prometheus 2.x. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5aa62345ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8935472c64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "info", | ||
| vec![ValueType::Vector, ValueType::Vector], | ||
| 1, | ||
| ValueType::Vector, |
There was a problem hiding this comment.
Validate info() second argument as selector syntax
Adding info as vec![ValueType::Vector, ValueType::Vector] without a dedicated AST check means any instant-vector expression is accepted as the optional second argument (for example, info(up, rate(http_requests_total[5m]))), even though Prometheus only allows a label-matcher selector in braces for this parameter. This causes the parser to accept queries that are invalid upstream, reducing compatibility and potentially breaking users when those queries are executed against real Prometheus implementations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
prometheus has a special check for info. will address this in next patch
| let expected_idx = if i < arg_types.len() { | ||
| i | ||
| } else { | ||
| arg_types.len() - 1 | ||
| }; |
There was a problem hiding this comment.
Found another difference:
Do we need to break if variadic is 0 when i >= arg_types.len()?
There was a problem hiding this comment.
this case should be checked by check_call_arity to ensure len(args) == len(arg_types) when variadic is 0
add a python script to sync function with upstream.
Added a few functions that were missing from our implementation.
breaking change: removed holt_winters to sync with upstream.