feat: jq support #1801
feat: jq support #1801
Conversation
github-actions
bot
added
the
type: feature
|
Warning Rate Limit Exceeded@ssddOnTop has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 18 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe updates primarily focus on integrating JAQ, a JSON transformation DSL, as an alternative to mustache templates across various modules. This involves adding new methods for evaluating JAQ filters, modifying mustache syntax, and updating related functionalities in structs and methods to support more complex JSON manipulations. Changes
Assessment against linked issues
The changes make significant strides towards integrating JAQ for JSON transformations, enhancing the flexibility and capabilities of the template system. However, some aspects like benchmarking and compile-time processing of JAQ templates need further clarification or implementation. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1801 +/- ##
==========================================
- Coverage 86.89% 86.60% -0.29%
==========================================
Files 156 156
Lines 15829 16078 +249
==========================================
+ Hits 13754 13924 +170
- Misses 2075 2154 +79 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Out of diff range and nitpick comments (1)
src/mustache.rs (1)
51-66: Clarify the behavior when the rendered value is empty.Consider adding documentation or unit tests to explain that
evaluateis used as a fallback when the rendered value is empty. This helps maintain clarity and ensures that future maintainers or users of this method understand its behavior.
|
| Report | Wed, May 1, 2024 at 13:52:15 UTC |
| Project | tailcall |
| Branch | 1801/merge |
| Testbed | benchmarking-runner |
Click to view all benchmark results
| Benchmark | Latency | Latency Results nanoseconds (ns) | (Δ%) | Latency Upper Boundary nanoseconds (ns) | (%) |
|---|---|---|---|
| group_by | ✅ (view plot) | 607.20 (-0.45%) | 635.42 (95.56%) |
| input/args.missing | ✅ (view plot) | 24.93 (-1.60%) | 26.23 (95.01%) |
| input/args.nested.existing | ✅ (view plot) | 56.13 (-0.18%) | 58.28 (96.31%) |
| input/args.nested.missing | ✅ (view plot) | 38.55 (-1.43%) | 39.98 (96.43%) |
| input/args.root | ✅ (view plot) | 53.22 (+0.66%) | 55.00 (96.76%) |
| input/headers.existing | ✅ (view plot) | 31.39 (-0.44%) | 31.81 (98.67%) |
| input/headers.missing | ✅ (view plot) | 30.05 (-1.39%) | 31.23 (96.21%) |
| input/value.missing | ✅ (view plot) | 23.09 (-2.26%) | 25.08 (92.04%) |
| input/value.nested.existing | ✅ (view plot) | 40.22 (-1.94%) | 42.78 (94.02%) |
| input/value.nested.missing | ✅ (view plot) | 35.25 (-1.45%) | 37.36 (94.36%) |
| input/value.root | ✅ (view plot) | 38.73 (+0.29%) | 39.85 (97.19%) |
| input/vars.existing | ✅ (view plot) | 8.36 (-1.15%) | 8.86 (94.33%) |
| input/vars.missing | ✅ (view plot) | 8.06 (-0.86%) | 8.95 (90.06%) |
| jq_bench | ✅ (view plot) | 2,302.80 | |
| mustache_bench | ✅ (view plot) | 279.30 | |
| test_batched_body | ✅ (view plot) | 1,776,700.00 (+2.71%) | 1,808,671.46 (98.23%) |
| test_data_loader | ✅ (view plot) | 473,150.00 (+0.16%) | 489,698.58 (96.62%) |
| with_mustache_expressions | ✅ (view plot) | 1,144.40 (-1.74%) | 1,217.95 (93.96%) |
| with_mustache_literal | ✅ (view plot) | 729.32 (+0.72%) | 760.15 (95.94%) |
Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help
There was a problem hiding this comment.
Actionable comments posted: 3
Out of diff range and nitpick comments (1)
src/mustache.rs (1)
Line range hint
44-53: Theparsemethod's fallback mechanism to treat the input as a literal if parsing fails is a good safeguard. However, consider logging the error for debugging purposes.+ Err(e) => { + log::error!("Failed to parse mustache template: {}", e); + Ok(Mustache::from(vec![Segment::Literal(str.to_string())])) + },
| fn parse_jq(input: &str) -> IResult<&str, Option<jaq_interpret::Filter>> { | ||
| let (input, _) = nom::character::complete::multispace0(input)?; | ||
| let (input, _) = tag("{{")(input)?; | ||
| let (input, _) = nom::character::complete::multispace0(input)?; | ||
| let (input, _) = tag("jq:")(input)?; | ||
| let (input, _) = nom::character::complete::multispace0(input)?; | ||
| let (input, filter) = take_until("}}")(input)?; | ||
| let filter = filter.trim(); | ||
| let mut defs = jaq_interpret::ParseCtx::new(vec![]); | ||
| defs.insert_natives(jaq_core::core()); | ||
| defs.insert_defs(jaq_std::std()); | ||
|
|
||
| let (filter, errs) = jaq_parse::parse(filter, jaq_parse::main()); | ||
| let _err_str = errs | ||
| .iter() | ||
| .map(|v| v.to_string()) | ||
| .collect::<Vec<String>>() | ||
| .join("\n"); | ||
| if !errs.is_empty() || filter.is_none() { | ||
| return Err(nom::Err::Error(nom::error::Error::new( | ||
| "failed to parse jq", // TODO: fix ownership issue and return _err_str | ||
| nom::error::ErrorKind::Tag, | ||
| ))); | ||
| } | ||
| let filter = defs.compile(filter.unwrap()); | ||
| Ok((input, Some(filter))) |
There was a problem hiding this comment.
The parse_jq function's error handling could be improved by including the actual error messages collected, which would aid in diagnosing parse failures.
+ "failed to parse jq", // TODO: fix ownership issue and return _err_str
+ _err_str,Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| fn parse_jq(input: &str) -> IResult<&str, Option<jaq_interpret::Filter>> { | |
| let (input, _) = nom::character::complete::multispace0(input)?; | |
| let (input, _) = tag("{{")(input)?; | |
| let (input, _) = nom::character::complete::multispace0(input)?; | |
| let (input, _) = tag("jq:")(input)?; | |
| let (input, _) = nom::character::complete::multispace0(input)?; | |
| let (input, filter) = take_until("}}")(input)?; | |
| let filter = filter.trim(); | |
| let mut defs = jaq_interpret::ParseCtx::new(vec![]); | |
| defs.insert_natives(jaq_core::core()); | |
| defs.insert_defs(jaq_std::std()); | |
| let (filter, errs) = jaq_parse::parse(filter, jaq_parse::main()); | |
| let _err_str = errs | |
| .iter() | |
| .map(|v| v.to_string()) | |
| .collect::<Vec<String>>() | |
| .join("\n"); | |
| if !errs.is_empty() || filter.is_none() { | |
| return Err(nom::Err::Error(nom::error::Error::new( | |
| "failed to parse jq", // TODO: fix ownership issue and return _err_str | |
| nom::error::ErrorKind::Tag, | |
| ))); | |
| } | |
| let filter = defs.compile(filter.unwrap()); | |
| Ok((input, Some(filter))) | |
| fn parse_jq(input: &str) -> IResult<&str, Option<jaq_interpret::Filter>> { | |
| let (input, _) = nom::character::complete::multispace0(input)?; | |
| let (input, _) = tag("{{")(input)?; | |
| let (input, _) = nom::character::complete::multispace0(input)?; | |
| let (input, _) = tag("jq:")(input)?; | |
| let (input, _) = nom::character::complete::multispace0(input)?; | |
| let (input, filter) = take_until("}}")(input)?; | |
| let filter = filter.trim(); | |
| let mut defs = jaq_interpret::ParseCtx::new(vec![]); | |
| defs.insert_natives(jaq_core::core()); | |
| defs.insert_defs(jaq_std::std()); | |
| let (filter, errs) = jaq_parse::parse(filter, jaq_parse::main()); | |
| let _err_str = errs | |
| .iter() | |
| .map(|v| v.to_string()) | |
| .collect::<Vec<String>>() | |
| .join("\n"); | |
| if !errs.is_empty() || filter.is_none() { | |
| return Err(nom::Err::Error(nom::error::Error::new( | |
| "failed to parse jq", // TODO: fix ownership issue and return _err_str | |
| _err_str, | |
| nom::error::ErrorKind::Tag, | |
| ))); | |
| } | |
| let filter = defs.compile(filter.unwrap()); | |
| Ok((input, Some(filter))) |
|
/tip 300$ |
|
🎉🎈 @ssddOnTop has been awarded $300! 🎈🎊 |
Issue Reference(s):
Fixes #1321
/claim #1321
Summary by CodeRabbit
New Features
Bug Fixes
Documentation