chore: remove temporal #2403
chore: remove temporal #2403
Conversation
TBonnin
force-pushed
the
tbonnin/remove-temporal
branch
3 times, most recently
from
717eb5e
to
57c544b
Compare
TBonnin
requested review from
bodinsamuel and
khaliqgant
and removed request for
bodinsamuel
TBonnin
force-pushed
the
tbonnin/remove-temporal
branch
from
57c544b
to
9526959
Compare
| logger.info('[autoidle] starting'); | ||
|
|
||
| const syncs = await findPausableDemoSyncs(); | ||
| const syncs = await findDemoSyncs(); |
There was a problem hiding this comment.
It is bit more complicated since schedules are not in the same database anymore and we cannot simply join syncs with schedules tables to find out which ones are running. We could limit to only search the syncs updated in the last X days so the size doesn't grow indefinitely over time, assuming this cron runs successfully at least once during this period. Can you think of a better idea? @khaliqgant @bodinsamuel
There was a problem hiding this comment.
yes make sense, not ideal but will scale for a long time
There was a problem hiding this comment.
A long term solution would be to add a pause_after feature into the orchestrator. For now I will keep the cron job
| const resSchedule = await orchestrator.deleteSync({ syncId: sync.id, environmentId: sync.environmentId }); | ||
| const deletedScheduleCount = resSchedule.isErr() ? 1 : 0; | ||
| logger.info(`[deleteSyncs] soft deleted ${deletedScheduleCount} schedules`); | ||
| metrics.increment(metrics.Types.JOBS_DELETE_SYNCS_DATA_SCHEDULES, deletedScheduleCount); | ||
|
|
There was a problem hiding this comment.
forgot this cron job. I will cleanup by hand the schedules that we are currently not deleting
| - 8081:8080 | ||
| networks: | ||
| - nango | ||
|
|
| logger.info('[autoidle] starting'); | ||
|
|
||
| const syncs = await findPausableDemoSyncs(); | ||
| const syncs = await findDemoSyncs(); |
There was a problem hiding this comment.
yes make sense, not ideal but will scale for a long time
TBonnin
force-pushed
the
tbonnin/remove-temporal
branch
2 times, most recently
from
d88076b
to
1a08f90
Compare
TBonnin
force-pushed
the
tbonnin/remove-temporal
branch
4 times, most recently
from
64b2456
to
f3a30c9
Compare
| if (res.isOk()) { | ||
| this.runningScripts.set(syncId, { ...scriptObject, cancelled: true }); | ||
| } else { | ||
| if (res.isErr()) { |
There was a problem hiding this comment.
fixing a race condition where the runner script execution could return before the runningScripts was updated, causing the runScript to return a non cancellation error
| if (task.abortController.signal.aborted) { | ||
| // task was aborted. No need to set it as failed | ||
| return; | ||
| } |
There was a problem hiding this comment.
We would attempt to set the already cancelled task to failed, which would fail. No need to do that
TBonnin
force-pushed
the
tbonnin/remove-temporal
branch
from
7ec4456
to
01629b4
Compare
Is it something you see in prod? This PR isn't intended to make any functional changes |
|
I'm seeing that locally, it's just a display thing not really critical |
Also remove any usage of the schedules table (table and data will be remove later)
TBonnin
force-pushed
the
tbonnin/remove-temporal
branch
from
01629b4
to
56a1668
Compare
what's in your nango.yaml? The UI should use the same value |
for example the one displayed as "every1h" is |
Also remove any usage of the schedules table (table and data will be remove later)
Issue ticket number and link
https://linear.app/nango/issue/NAN-1222/remove-temporal-from-codebase
https://linear.app/nango/issue/NAN-1091/update-documentation-about-temporal
Checklist before requesting a review (skip if just adding/editing APIs & templates)