feat(catalog-etl): consolidate terminal ETL scheduler into single sequential handler#289
Open
GitAddRemote wants to merge 1 commit into
Open
feat(catalog-etl): consolidate terminal ETL scheduler into single sequential handler#289GitAddRemote wants to merge 1 commit into
GitAddRemote wants to merge 1 commit into
Conversation
…uential handler - Replace two independent @Cron methods (terminals-sync at :00, terminal-distances-sync at :05) with a single scheduledTerminalEtl() at '0 * * * *' - terminals-sync runs first; abort on failure or ConflictException before attempting distances - ConflictException from either step logs debug (not error) and returns cleanly - Add shouldSkip() guard for terminal-distances-sync so a recent run is respected independently - Add 6-case spec covering sequential success, abort-on-failure, conflict handling, distances failure, and per-step skip guards Closes #221
1 task
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates the terminal-related Catalog ETL scheduling logic into a single hourly cron handler to enforce step ordering in code (terminals first, then distances) and standardize error/lock handling behavior.
Changes:
- Replaced two independent
@Cronhandlers with onescheduledTerminalEtl()running at0 * * * *. - Implemented sequential execution with early abort on
terminals-syncfailure and debug-only handling forConflictException. - Added unit tests covering the combined sequential scheduler behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/src/modules/catalog-etl/schedulers/catalog-etl.scheduler.ts | Consolidates terminal ETL cron scheduling into a single sequential handler and updates logging/error handling. |
| backend/src/modules/catalog-etl/schedulers/catalog-etl.scheduler.spec.ts | Adds unit coverage for the new sequential scheduler behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+17
to
+20
| async scheduledTerminalEtl(): Promise<void> { | ||
| if (await this.shouldSkip('terminals-sync')) return; | ||
| this.logger.info('Starting scheduled terminals sync'); | ||
| this.logger.info('Starting scheduled terminal ETL'); | ||
|
|
Comment on lines
+68
to
+72
| it('skips when terminals-sync was completed within SKIP_HOURS', async () => { | ||
| mockGetLastSuccessfulStepRun.mockResolvedValue(new Date().toISOString()); | ||
| await makeScheduler().scheduledTerminalEtl(); | ||
| expect(mockRunStep).not.toHaveBeenCalled(); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@Cronmethods (scheduledTerminalsSyncat:00andscheduledTerminalDistancesSyncat:05) with a singlescheduledTerminalEtl()at'0 * * * *'terminals-syncruns first; any failure (error orConflictException) aborts before attemptingterminal-distances-sync, preventing wasted work on a partial stateConflictExceptionfrom either step is treated as a non-error condition: logsdebugand returns cleanly — noerrorlog noiseshouldSkip()is called independently for each step so a recently-completedterminal-distances-syncis still respected even whenterminals-syncwas not skippedTest plan
Closes #221