refactor: remove legacy cron-only deal and retrieval batch methods#363
refactor: remove legacy cron-only deal and retrieval batch methods#363silent-cipher wants to merge 15 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Removes legacy “all providers / batch” methods that were only used by the old cron scheduler, continuing the cleanup/migration to pg-boss–managed per-provider orchestration.
Changes:
- Removed cron-only batch retrieval orchestration helpers from
RetrievalService. - Removed “create deals for all providers” flow from
DealServiceand dev-tools API/service. - Deleted corresponding unit tests that targeted the removed cron-only behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/backend/src/retrieval/retrieval.service.ts | Removes batch retrieval selection + parallel processing helpers. |
| apps/backend/src/retrieval/retrieval.service.spec.ts | Removes tests for the deleted parallel batch helper. |
| apps/backend/src/dev-tools/dev-tools.service.ts | Removes dev-tools “all providers” deal creation trigger. |
| apps/backend/src/dev-tools/dev-tools.controller.ts | Removes the /api/dev/deals/create-all endpoint. |
| apps/backend/src/deal/deal.service.ts | Removes “create deals for all providers” orchestration + parallel helper. |
| apps/backend/src/deal/deal.service.spec.ts | Removes tests for deleted orchestration and loosens dataset metadata assertion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| private isPgBossMode(): boolean { | ||
| return (this.configService.get("jobs")?.mode ?? "cron") === "pgboss"; |
There was a problem hiding this comment.
@silent-cipher I think this one definitely needs addressed fully since we no longer have a "jobs.mode", this will always be false as it stands currently
SgtPooki
left a comment
SgtPooki
left a comment
There was a problem hiding this comment.
lgtm.. didn't see anything major
| * starts pg-boss, registers workers, and starts the scheduler polling loop. | ||
| */ | ||
| async onModuleInit(): Promise<void> { | ||
| if (!this.isPgbossEnabled()) { |
There was a problem hiding this comment.
aah I rewrote this function after deleting it earlier, so the rename slipped in unintentionally in #355
Would you prefer I revert the naming change in this PR or handle it in a follow-up PR?
| const jobsConfig = this.configService.get("jobs", { infer: true }); | ||
| const scheduling = this.configService.get("scheduling", { infer: true }); |
There was a problem hiding this comment.
what does infer: true do here?
There was a problem hiding this comment.
infer set to true -> will automatically infer the property type based on config interface
Built on top of #355
Another setup towards #271