feat: normalize durations and timestamps #837
Conversation
taddes
changed the title
taddes
force-pushed
the
feat/normalize_timestamps-SYNC-4521
branch
2 times, most recently
from
e1d4a9c to
5c59883
Compare
taddes
force-pushed
the
feat/normalize_timestamps-SYNC-4521
branch
from
5c59883 to
d89e5ed
Compare
taddes
changed the title
| @@ -64,7 +64,7 @@ log = { version = "0.4", features = [ | |||
| ] } | |||
| mockall = "0.12" | |||
| mozsvc-common = "0.2" | |||
| openssl = { version = "0.10" } | |||
| openssl = { version = "0.10.70" } | |||
There was a problem hiding this comment.
We tend to not specify minor revisions here. It's a good balance of not having to touch Cargo.toml for every minor bump + allows cargo to figure out the latest version for us when needed -- which shouldn't matter to our code as minor revisions should be backwards compat/not change APIs.
To handle CVE/RUSTSECs in this way you can usually just do an e.g. cargo update -p openssl to bump to the latest 0.10 only in Cargo.lock
There was a problem hiding this comment.
IIRC, Rust is a bit odd about versions, so
0.10.70 translates to "use anything between 0.10.70 and 0.11.0" (which is the same as ~0.10.70 and because there's a leading 0 it's special cased.)
If we need to lock to a specific version, we should use ^0.10.70
There was a problem hiding this comment.
Cool, thx for that! Will do.
| @@ -46,10 +48,10 @@ pub mod util; | |||
| /// "abandoned" and any router info assigned to a User Agent that has not contacted | |||
| /// Autopush in 60 days can be discarded. | |||
| /// | |||
| const ONE_DAY_IN_SECONDS: u64 = 24 * 60 * 60; | |||
| pub const ONE_DAY_IN_SECONDS: u64 = Duration::days(1).num_seconds() as u64; | |||
There was a problem hiding this comment.
Huh. I thought the idea was that we'd make these Duration values, and only convert to seconds when we need to.
e.g.
.map(|ttl| min(ttl, MAX_NOTIFICATION_TTL.num_seconds() as i64)
Basically, strongly type TTL as a Duration so that we don't muck up how we work with them accidentally.
There was a problem hiding this comment.
I believe Taddes is working on that (so this is currently a WIP)
There was a problem hiding this comment.
Ah, sorry! Hope you don't mind, but I converted this to a draft for now so I don't do more dumb drive-by's.
My apologies.
There was a problem hiding this comment.
Yeah, sry, should have moved back to WIP. Wrestling with Prometheus all day and didn't put it back.
Closes SYNC-4521