Repository: prometheus/common
Pull request #226 (2020-03-23 14:52:36)

Support 0 for model.Duration.
This allows to pass the string "0" to parse a model.Duration. This is because `time.Duration` also support this zero value and so it is easier to interchange model.Duration with time.Duration.

time.Duration does parses 0 using this:

```
	// Special case: if all that is left is "0", this is zero.
	if s == "0" {
		return 0, nil
	}
```

see https://golang.org/src/time/format.go?s=40541:40587#L1389

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

brian-brazil: We always require a non-zero value when this is used in Prometheus, so I think requiring a unit is reasonable.
cyriltovena: Ok !
gouthamve: > We always require a non-zero value when this is used in Prometheus

This is not true, we can now set the retention duration as `0s`. I'd say this is a very obscure message and can be made better with this PR:

```
➜  prometheus git:(master) ./prometheus --storage.tsdb.retention.time=0 --storage.tsdb.retention.size=1GB 
Error parsing commandline arguments: not a valid duration string: "0"
usage: prometheus [<flags>]
```
brian-brazil: > This is not true, we can now set the retention duration as 0s.

I'd argue that that's not a sane setup, as you'll still have 2 hours in the head.
gouthamve: Nope, this is when I want only size based retention, and not time based. The way to disable time based is to set it to 0, if I'm not wrong?
brian-brazil: That's true.
beorn7: Note: https://golang.org/pkg/time/#ParseDuration also doesn't need a unit iff the value is "0".
cyriltovena: > Note: https://golang.org/pkg/time/#ParseDuration also doesn't need a unit iff the value is "0".

Yeah that's what I was trying to explain in the PR description.
beorn7: You did @cyriltovena . I have to apologize for my limited attention span. (Pretending to be a millennial… :o)
brian-brazil: Can you add the DCO?
beorn7: Everyone seems happy now. Merging…

Patch:
diff --git a/model/time.go b/model/time.go
index 7b0064f..490a024 100644
--- a/model/time.go
+++ b/model/time.go
@@ -186,6 +186,10 @@ var durationRE = regexp.MustCompile("^([0-9]+)(y|w|d|h|m|s|ms)$")
 // ParseDuration parses a string into a time.Duration, assuming that a year
 // always has 365d, a week always has 7d, and a day always has 24h.
 func ParseDuration(durationStr string) (Duration, error) {
+	// Allow 0 without a unit.
+	if durationStr == "0" {
+		return 0, nil
+	}
 	matches := durationRE.FindStringSubmatch(durationStr)
 	if len(matches) != 3 {
 		return 0, fmt.Errorf("not a valid duration string: %q", durationStr)


Test patch:
diff --git a/model/time_test.go b/model/time_test.go
index 315bc4e..330cc6e 100644
--- a/model/time_test.go
+++ b/model/time_test.go
@@ -90,8 +90,14 @@ func TestParseDuration(t *testing.T) {
 	var cases = []struct {
 		in  string
 		out time.Duration
+
+		expectedString string
 	}{
 		{
+			in:             "0",
+			out:            0,
+			expectedString: "0s",
+		}, {
 			in:  "0s",
 			out: 0,
 		}, {
@@ -126,7 +132,11 @@ func TestParseDuration(t *testing.T) {
 		if time.Duration(d) != c.out {
 			t.Errorf("Expected %v but got %v", c.out, d)
 		}
-		if d.String() != c.in {
+		expectedString := c.expectedString
+		if c.expectedString == "" {
+			expectedString = c.in
+		}
+		if d.String() != expectedString {
 			t.Errorf("Expected duration string %q but got %q", c.in, d.String())
 		}
 	}