feat(stdlib): allow parsing multiple durations at once in `parse_duration` function #1197

titaneric · 2024-12-27T18:59:12Z

Summary

Hi, I hope that parse_duration could parse multiple duration at once, so I have opened #1198 as a referenced issue.

Change Type

Bug fix
New feature
Non-functional (chore, refactoring, docs)
Performance

Is this a breaking change?

Yes
No

How did you test this PR?

cargo test
./scripts/check.sh

Does this PR include user facing changes?

Yes. Please add a changelog fragment based on
our guidelines.
No. A maintainer will apply the "no-changelog" label to this PR.

Checklist

Our CONTRIBUTING.md is a good starting place.
If this PR introduces changes to LICENSE-3rdparty.csv, please
run dd-rust-license-tool write and commit the changes. More details here.
For new VRL functions, please also create a sibling PR in Vector to document the new function.

References

Closes #1196

pront

Thanks @titaneric! I left a few comments. The most interesting one is around the RE change.

pront · 2025-01-02T16:25:40Z

changelog.d/1198.feature.md

@@ -0,0 +1,3 @@
+Added `parse_bytes` function to parse bytes either from SI or binary unit depending on the given `base` argument.


Please git rm this file.

pront · 2025-01-02T16:27:31Z

changelog.d/1197.feature.md

@@ -0,0 +1 @@
+Support multiple-units duration string for `parse_duration` function such as `1h2s`.


Suggested change

Support multiple-units duration string for `parse_duration` function such as `1h2s`.

Added support for multi-unit duration strings (e.g., `1h2s`, `2m3s`) in the `parse_duration` function.

pront · 2025-01-02T16:30:30Z

src/stdlib/parse_duration.rs

+        let captures = RE
+            .captures(value)
+            .ok_or(format!("unable to parse duration: '{value}'"))?;
+        let capture_match = captures.get(0).unwrap();


Please do not use unwrap(), we can return an error here.

pront · 2025-01-02T16:30:58Z

src/stdlib/parse_duration.rs

-        .to_f64()
-        .ok_or(format!("unable to format duration: '{number}'"))?;
-    Ok(Value::from_f64_or_zero(number))
+    let mut num = 0.0;


Nit:

Suggested change

let mut num = 0.0;

let mut sum = 0.0;

pront · 2025-01-02T16:34:35Z

src/stdlib/parse_duration.rs

 }

 static RE: Lazy<Regex> = Lazy::new(|| {
    Regex::new(
        r"(?ix)                        # i: case-insensitive, x: ignore whitespace + comments
-            \A


I noticed you remove the anchors here, I suppose this is needed for iterative parsing. What are the implications of this? Would this function now accept strings like 1h2s extrablah?

I wonder if we can leverage https://docs.rs/humantime/latest/humantime/ here without breaking existing behavior.

Yes, I remove it since I want to iterate the regex matching.
The duration 1h2s extrablah would not be accepted, and its error is Err("unable to parse duration: ' extrablah'")

humantime seems promising for me! I would try it in the next commit.

pront

Thank you @titaneric, this function is now more powerful than before and easier to maintain.

pront · 2025-01-03T15:11:02Z

src/stdlib/parse_duration.rs

-        .to_f64()
-        .ok_or(format!("unable to format duration: '{number}'"))?;
+    let duration = ht_parse_duration(&trimmed_value)
+        .map_err(|_| format!("unable to parse duration: '{value}'"))?;


Suggested change

.map_err(|_| format!("unable to parse duration: '{value}'"))?;

.map_err(|e| format!("unable to parse duration: '{e}'"))?;

This will require updates to the test cases.

pront · 2025-01-03T15:13:30Z

Note: there are multiple failing CI checks, you reproduce them locally with ./scripts/checks.sh.

titaneric · 2025-01-03T15:50:53Z

A little awkward... We need to update MSRV to 1.80 to support Duration::div_duration_f64 method which is a BREAKING change.

There is another Duration::div_f64 method which MSRV is 1.38, but I need to change the implementation a little bit and it's hard to maintain.

pront · 2025-01-03T16:28:14Z

A little awkward... We need to update MSRV to 1.80 to support Duration::div_duration_f64 method which is a BREAKING change.

There is another Duration::div_f64 method which MSRV is 1.38, but I need to change the implementation a little bit and it's hard to maintain.

No worries, we can bump the MSRV. Do you mind doing the bump in a separate PR please?

titaneric · 2025-01-03T16:51:55Z

ok, I could raise another PR to bump the MSRV

titaneric · 2025-01-03T18:36:43Z

I've rebased the main branch and update the LICENSE, please check!

pront · 2025-01-03T19:49:32Z

Thank you @titaneric, this looks great.

titaneric changed the title ~~feat(stdlib): allow multi-durations in parse_duration function~~ feat(stdlib): allow parsing multiple durations at once in parse_duration function Dec 28, 2024

This was referenced Dec 28, 2024

feat(stdlib) Add new parse_bytes function #1198

Open

docs(vrl): add more parse-duration function doc vectordotdev/vector#22088

Open

pront self-assigned this Jan 2, 2025

pront reviewed Jan 2, 2025

View reviewed changes

pront approved these changes Jan 3, 2025

View reviewed changes

titaneric mentioned this pull request Jan 3, 2025

Bump MSRV to 1.80 #1204

Merged

11 tasks

titaneric added 12 commits January 4, 2025 02:21

Allow multiple units duration

fe26d59

Borrow values from slice

e6b74d8

do not allow space in given duration

61bd177

Remove redundant reference after script check

7f7f5bf

Add more testcases

1dbb35f

Add changelog

53d8c34

Add changelog

ed01e8b

Tidy up changelog

a07824a

Remove unwrap and rename sum var

d5f84fa

Remove accidentally added changelog

59318f4

adopt humantime crate for paring duration

4522dd8

Map error from crate

35dc8e2

titaneric force-pushed the feat/multi-durations branch from ea72949 to 35dc8e2 Compare January 3, 2025 18:29

Apply dd-rust-license-tool write to update LICENSE

cf730ec

pront added this pull request to the merge queue Jan 3, 2025

Merged via the queue into vectordotdev:main with commit 3c82c74 Jan 3, 2025
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(stdlib): allow parsing multiple durations at once in `parse_duration` function #1197

feat(stdlib): allow parsing multiple durations at once in `parse_duration` function #1197

titaneric commented Dec 27, 2024 •

edited

Loading

pront left a comment

pront Jan 2, 2025

pront Jan 2, 2025

pront Jan 2, 2025

pront Jan 2, 2025

pront Jan 2, 2025

titaneric Jan 2, 2025

pront left a comment

pront Jan 3, 2025 •

edited

Loading

pront commented Jan 3, 2025

titaneric commented Jan 3, 2025 •

edited

Loading

pront commented Jan 3, 2025

titaneric commented Jan 3, 2025

titaneric commented Jan 3, 2025

pront commented Jan 3, 2025

		@@ -0,0 +1,3 @@
		Added `parse_bytes` function to parse bytes either from SI or binary unit depending on the given `base` argument.

		@@ -0,0 +1 @@
		Support multiple-units duration string for `parse_duration` function such as `1h2s`.

	Support multiple-units duration string for `parse_duration` function such as `1h2s`.
	Added support for multi-unit duration strings (e.g., `1h2s`, `2m3s`) in the `parse_duration` function.

	.map_err(\|_\| format!("unable to parse duration: '{value}'"))?;
	.map_err(\|e\| format!("unable to parse duration: '{e}'"))?;

feat(stdlib): allow parsing multiple durations at once in parse_duration function #1197

feat(stdlib): allow parsing multiple durations at once in parse_duration function #1197

Conversation

titaneric commented Dec 27, 2024 • edited Loading

Summary

Change Type

Is this a breaking change?

How did you test this PR?

Does this PR include user facing changes?

Checklist

References

pront left a comment

Choose a reason for hiding this comment

pront Jan 2, 2025

Choose a reason for hiding this comment

pront Jan 2, 2025

Choose a reason for hiding this comment

pront Jan 2, 2025

Choose a reason for hiding this comment

pront Jan 2, 2025

Choose a reason for hiding this comment

pront Jan 2, 2025

Choose a reason for hiding this comment

titaneric Jan 2, 2025

Choose a reason for hiding this comment

pront left a comment

Choose a reason for hiding this comment

pront Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

pront commented Jan 3, 2025

titaneric commented Jan 3, 2025 • edited Loading

pront commented Jan 3, 2025

titaneric commented Jan 3, 2025

titaneric commented Jan 3, 2025

pront commented Jan 3, 2025

feat(stdlib): allow parsing multiple durations at once in `parse_duration` function #1197

feat(stdlib): allow parsing multiple durations at once in `parse_duration` function #1197

titaneric commented Dec 27, 2024 •

edited

Loading

pront Jan 3, 2025 •

edited

Loading

titaneric commented Jan 3, 2025 •

edited

Loading