interagent/pliny

Add zulu time formatting helpers

ransombriggs opened this issue · 3 comments

ransombriggs commented

To get Time objects into a zulu formatted string various pliny apps have defined identical helpers that could be consolidated in pliny. I started this issue rather than filing a PR since there are some approach questions that we should probably discuss.

The time_v3 helpers are usually defined in a module which is mixed into serializers and sometimes mediators if they are publishing events. There are currently two different implementations I have seen.

def time_v3(time)
  time&.getutc&.strftime("%Y-%m-%dT%H:%M:%SZ")
end

def time_v3(time, precision: 0)
  time&.getutc&.iso8601(precision)
end

The second version has the extra flexibility of having sub second precisions which I believe the app uses for event timestamps. The only problem with this method is that it does not work for DateTime objects where as the first implementation does.

[1] development(main)> Time.now.getutc.iso8601
=> "2017-11-29T16:45:40Z"
[2] development(main)> DateTime.now.getutc.iso8601
=> "2017-11-29T16:45:45+00:00"

I guess I would lobby for the strftime implementation at this point so that it would not be confusing when used with DateTime objects.

Also, time_v3 is probably not the best name for this method in pliny since it is tied to the idea of the heroku api v3 implementation instead of zulu formatted dates.

Finally, would this be better as an optional mixin on Time and DateTime instead?

/cc @gudmundur

gudmundur commented

@ransombriggs Thanks for the thorough details on this issue.

As for which implementation to use, it's a tough one. I'm not sure what the reasoning behind the time precision is, maybe @rwz can get into that? As Pliny defaults to using DateTime in serializers, I'd say we should stick to that. Higher precision can be provided for the event envelope specifically.

I agree that the time_v3 name isn't the best, how about time or zulu_time?

I think we should leave it as a class method on the serializer base class, rather than mixing into Time or DateTime.

ransombriggs commented

@gudmundur I am down with having it available as a class method on the serializer base class, but would like to continue leaving it as a module that I mix into the serializer base. I have seen usage in mediators as well, specifically for submitting jobs with a string timestamp and would like to explicitly include the helper there.