3scale / apisonator

Red Hat 3scale API Management Apisonator backend
https://3scale.net
Apache License 2.0
35 stars 27 forks source link

Transactions with UNIX timestamps #167

Closed unleashed closed 4 years ago

unleashed commented 4 years ago

This adds support for specifying transaction timestamps as UNIX Epoch timestamps, ie. number of seconds since Jan 1, 1970.

unleashed commented 4 years ago

So... 2012garbage2012 is an invalid date, but '201210garbage201210' is totally valid, no questions asked.

davidor commented 4 years ago

What? :dizzy_face:

unleashed commented 4 years ago

What? dizzy_face

fliptableagain

unleashed commented 4 years ago

So I think the only reasonable way out of this surprising legacy from the past, especially given this is behaviour is not documented, would be to remove this offending test, since it does not make sense any way to do the negative testing (and even then it was so broken), and accept (for now) the discovery that completely broken timestamps like the one I specified can actually mean some random thing to the system, just the same way it's always been, and then consider whether to fix what was already broken adding validation.

Ideas?

davidor commented 4 years ago

@unleashed That sounds good to me :+1:

unleashed commented 4 years ago

So in the end I added a pending test that is way more likely to bug people when running the suite to go and fix this issue one way or another, and also changed the "negative" testing to look for changes in 201201.

davidor commented 4 years ago

Is there something we can do about that? Have you taken a look to see if we can replace that Date._parsecall with a similar call that does not cause other issues?

@unleashed It'd be good to open an issue so we remember to address that.

unleashed commented 4 years ago

Is there something we can do about that? Have you taken a look to see if we can replace that Date._parsecall with a similar call that does not cause other issues?

Maybe, but would certainly change more semantics - we can defer that to another PR.