Page MenuHomePhabricator

[keyserver] Add param to getRetention to customize activity period being checked
ClosedPublic

Authored by ashoat on Mon, Nov 25, 11:59 AM.
Tags
None
Referenced Files
F3397162: D14040.diff
Sun, Dec 1, 4:14 PM
Unknown Object (File)
Sat, Nov 30, 6:01 AM
Unknown Object (File)
Fri, Nov 29, 1:49 PM
Unknown Object (File)
Tue, Nov 26, 3:34 AM
Subscribers

Details

Summary

We want to introduce new metrics that check retention based on usage in the last week rather than usage in the last day. This will be handled in the next diff, but first we want to factor out the param.

Test Plan

Ran it on my personal server and confirmed that a message appeared with the correct metrics

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun added inline comments.
keyserver/src/cron/metrics.js
78 ↗(On Diff #46040)
100 ↗(On Diff #46040)

shouldn't we rename this variable? maybe xDaysAgo or something

111 ↗(On Diff #46040)

should this param default to 1 to match the function description?

This revision is now accepted and ready to land.Mon, Nov 25, 2:02 PM
keyserver/src/cron/metrics.js
111 ↗(On Diff #46040)

actually we probably want to just update the code comment above. specifically the second sentence

This revision was landed with ongoing or failed builds.Mon, Nov 25, 2:25 PM
This revision was automatically updated to reflect the committed changes.