knative / eventing

Event-driven application platform for Kubernetes
https://knative.dev/docs/eventing
Apache License 2.0
1.42k stars 597 forks source link

DeliverySpec.backoffDelay does not accept RFC3339 duration, but timestamp #3555

Closed FWiesner closed 4 years ago

FWiesner commented 4 years ago

https://github.com/knative/eventing/blob/a6b7494a5b8fbb2e9574dfbe54fbb9dad53ad121/pkg/apis/duck/v1/delivery_types.go#L70

It was possible to set 2019-10-12T07:20:50.52Z as value, but not P1S

FWiesner commented 4 years ago

time.ParseDuration() uses a custom format, not RFC3339 and time.Parse(...) parses timestamps. You might need https://github.com/peterhellberg/duration/blob/master/duration.go or similar

grantr commented 4 years ago

@lionelvillard What was the intent for valid values for this field?

lionelvillard commented 4 years ago

The intent is to use the ISO8601 duration format which is not covered by RFC3339 (only mentioned in the Appendix A).

We need to fix our doc and implementation.

Thanks @FWiesner for reporting this!

FWiesner commented 4 years ago

btw what is the default value?

lionelvillard commented 4 years ago

AFAIK, there is no channel implementations supporting retries and there is no predefined default values.

FWiesner commented 4 years ago

@lionelvillard I've heard repeatedly the KafkaChannel would

slinkydeveloper commented 4 years ago

Seems like in golang stdlib there is no way to define the format to parse a duration... https://github.com/knative/eventing/issues/3555

pierDipi commented 4 years ago

/assign

slinkydeveloper commented 4 years ago

There is this library https://github.com/senseyeio/duration/blob/master/duration.go which implements this library but then uses its own Duration type more than the one in the stdlib. We might take inspiration for the parsing part, but then use the golang stdlib type... wdyt @pierDipi ?

pierDipi commented 4 years ago

This lib seems maintained https://github.com/rickb777/date (latest release 4 days ago), and also, it supports converting the parsed duration to the standard time.Duration.

slinkydeveloper commented 4 years ago

cool, then sgtm!