koden-km commented 6 years ago

Fixes #94

koden-km commented 6 years ago

I'm not sure about using time.Now(). Should a time value be passed into the functions instead? Should I just use MySQL's NOW() directly instead of getting time from Go and formatting?

codecov-io commented 6 years ago

Codecov Report

Merging #96 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #96   +/-   ##
=======================================
  Coverage   31.51%   31.51%           
=======================================
  Files          66       66           
  Lines        2110     2110           
=======================================
  Hits          665      665           
  Misses       1435     1435           
  Partials       10       10

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f26a40e...a62e154. Read the comment docs.

jmalloc commented 6 years ago

You don't even need to pass the value, actually. For all of these times, we can use a regular TIMESTAMP(6) column. It's probably not obvious, but I've used regular timestamps where the system is just tracking when some DB operation happens, and used a VARCHAR with the RFC-3339 encoding only when I wanted to preserve the timestamp / timezone exactly as it appeared inside some message.

I think the saga_instance table should actually have 2 timestamps - one for insert, and another for update. You can use DEFAULT and ON UPDATE clauses to make this totally automatic:

insert_time TIMESTAMP(6) DEFAULT CURRENT_TIMESTAMP(6),
update_time TIMESTAMP(6) DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6),

koden-km commented 6 years ago

Updated as suggested.

jmalloc / ax

Store a timestamp on saga instances and snapshots #96

Codecov Report