Page MenuHomePhabricator

[keyserver] Bump `node-schedule` to `2.1.0`
ClosedPublic

Authored by atul on Apr 27 2022, 7:57 AM.
Tags
None
Referenced Files
F2779347: D3858.id12060.diff
Fri, Sep 20, 10:26 AM
F2779346: D3858.id11988.diff
Fri, Sep 20, 10:25 AM
F2778973: D3858.diff
Fri, Sep 20, 8:27 AM
F2765389: D3858.diff
Thu, Sep 19, 2:25 PM
Unknown Object (File)
Fri, Sep 13, 7:44 PM
Unknown Object (File)
Mon, Sep 2, 11:30 PM
Unknown Object (File)
Wed, Aug 28, 1:37 PM
Unknown Object (File)
Wed, Aug 28, 3:17 AM

Details

Summary

Downstream dependencies have issues surfaced by yarn audit

Checked the release notes: https://github.com/node-schedule/node-schedule/releases and there don't seem to be any breaking changes that affect us:

It looks like the two breaking changes since are

  1. Drop support for Node < 6

which doesn't affect us

  1. "This removes support for passing job objects with an execute() method instead of a function... This is technically a breaking change, although this "feature" was never documented."

which doesn't affect us either. checked cron.js which is the only place we use node-schedule and we aren't using the execute() approach anywhere


Depends on D3857

Test Plan

CI

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul requested review of this revision.Apr 27 2022, 8:02 AM
  1. Please make sure to make me a blocking reviewer on diffs that affect dependencies in the future
  2. Can you also test wherever we use node-schedule to make sure it still works as expected? You can adjust the schedule of some cronjob to make it run every minute or something
This revision is now accepted and ready to land.Apr 27 2022, 2:31 PM

Please make sure to make me a blocking reviewer on diffs that affect dependencies in the future

Was planning on adding you as blocking reviewer before landing if @palys-swm had accepted first. Can add you as blocking reviewer immediately in the future.

No downside to adding me as a blocking reviewer right away, I think

No downside to adding me as a blocking reviewer right away, I think

Makes sense. I thought one of the goals was to reduce the size of your "Must Review" queue, but I must have been mistaken. Did add you as first-pass reviewer though so it'd show up in "Ready to Review."

I thought one of the goals was to reduce the size of your "Must Review" queue, but I must have been mistaken

Nah that's not really a goal I think... but I see where you're coming from now

Can you also test wherever we use node-schedule to make sure it still works as expected? You can adjust the schedule of some cronjob to make it run every minute or something

We exclusively use node-schedule in cron.js. I tested this by logging "CRON" every minute and verifying that it appeared in the keyserver output. I think this is a reasonable indication that node-schedule is still working as expected? Open to any other suggestions if it's not.

9cfc.png (462×1 px, 383 KB)

This revision was landed with ongoing or failed builds.Apr 28 2022, 11:55 AM
This revision was automatically updated to reflect the committed changes.