-
Notifications
You must be signed in to change notification settings - Fork 198
Description
Module: misk-cron
Context
In 2021 in 2e60d31#diff-c0a47ac7efd8e71eeae297ce15149f4aa41d51751b9a6e4be12af6f34362bc89, Scotte Zinn introduced the CronModule and FakeCronModule. I now believe that the 'Fake' is a misnomer. It seems as if Scotte intended to either inline the `FakeCronModule, rename it, or further split apart the real and fake dependencies.
My first clue is that the real CronModule installs the FakeCronModule. It is atypical for real modules to depend on fake modules.
The FakeCronModule installs an ExecutorServiceModule such that the Guice dependency map includes an ExecutorService. However that ExecutorService is unused – nothing installed via FakeCronModule alone is using the ExecutorService to execute anything. It can be observe in the logs that CronManager adds each of the expected cron entries, but no logs from misk-cron-cronjob-%d occur and the individual CronRunnableEntry instances are not invoked.
The real CronModule provides a RepeatedTaskQueue which corresponds to the fake's ExecutorService via the @ForMiskCron annotation.
Proposed Solution
The FakeCronModule should be removed and its configuration inlined into the real CronModule. This will be a breaking change to users of the FakeCronModule.
In my opinion this is a valuable breaking change, as users of the FakeCronModule are likely not experiencing the behavior the expect.