Bot cron job Part 1/2 #1743
Bot cron job Part 1/2 #1743
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
As part of this PR, can you add a short section on our "Bot basics" page to indicate that Bos can be run on cron timers (under the "Executing a Bot" section)? https://www.medplum.com/docs/bots/bot-basics#executing-a-bot
There was a problem hiding this comment.
Nice job @jamestouri ! Structurally, this looks very solid, and I think it's close to ready for beta testing.
The main feedback I have on the PR as it stands is to rename scheduledtiming.ts to cron.ts and rename the exported methods accordingly.
There are a few other nit comments.
Then there is one more thing we need to do before we can push to prod: We should add a feature flag check inside addScheduledTimingJobs / addCronJobs for Project.features.
Here is an example of how we do feature flag checks: https://github.com/medplum/medplum/blob/main/packages/server/src/fhir/operations/execute.ts#L129-L137
export async function isBotEnabled(bot: Bot): Promise<boolean> {
const project = await systemRepo.readResource<Project>('Project', bot.meta?.project as string);
return !!project.features?.includes('bots');
}We can introduce a similar feature flag named cron for this.
Otherwise, this looks great, and will be an awesome feature.
| queue = new Queue<ScheduledTimingJobData>(queueName, { | ||
| ...defaultOptions, | ||
| defaultJobOptions: { | ||
| attempts: 3, |
There was a problem hiding this comment.
Oy, retry policy. This seems like a very sensible default, and a good place to start 👍
I filed a separate ticket to track this: #1749
| if (resource.cronTiming) { | ||
| cron = convertTimingToCron(resource.cronTiming); | ||
| if (!cron) { | ||
| logger.debug('cronTiming had the wrong format for a timed cron job'); |
There was a problem hiding this comment.
This is a good design question -- how can we provide feedback about invalid cron expressions.
I think this is an ok starting point, because we're still early in the cron development.
In the future, we may want to explore leveraging capabilities in ElementDefinition.constraint to make it a write-time check: https://hl7.org/fhir/elementdefinition-definitions.html#ElementDefinition.constraint
There was a problem hiding this comment.
Just filed a ticket to track this: #1751
jamestouri
force-pushed
the
james-bot-cron-job
branch
from
1d16faf
to
11e59be
Compare
|
Kudos, SonarCloud Quality Gate passed! |
Here is an example of a bot scheduled on the cron job. It was running every minute, then stopped it, then ran every minute again.
Next steps after PR