refactor(admin-api): generalize scheduler trait over job kinds#1842
refactor(admin-api): generalize scheduler trait over job kinds#1842
Conversation
LNSD
force-pushed
the
lnsd/refactor-admin-api-job-kinds
branch
5 times, most recently
from
5c143ed to
17cc612
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on March 21
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| let job_descriptor = if dataset.kind() == DerivedDatasetKind { | ||
| MaterializeDerivedDatasetJobDescriptor { | ||
| end_block: end_block.into(), | ||
| dataset_namespace: reference.namespace().clone(), | ||
| dataset_name: reference.name().clone(), | ||
| manifest_hash: reference.hash().clone(), | ||
| } | ||
| .into() | ||
| } else { | ||
| MaterializeRawDatasetJobDescriptor { | ||
| end_block: end_block.into(), | ||
| max_writers: parallelism, | ||
| dataset_namespace: reference.namespace().clone(), | ||
| dataset_name: reference.name().clone(), | ||
| manifest_hash: reference.hash().clone(), | ||
| } | ||
| .into() | ||
| }; |
There was a problem hiding this comment.
Can we move this logic to a JobDescriptor::materialise_dataset() fn ?
There was a problem hiding this comment.
Keeping this in the handler intentionally.
JobDescriptor is a thin, type-erased bridge. It shouldn't know about MaterializeRawDatasetJobDescriptor or MaterializeDerivedDatasetJobDescriptor internal details. Construction must use the actual typed worker descriptors at the call site where the dataset-kind context lives.
Eventually, these will be extracted directly from the request body, so the handler is the right place for this logic.
Separate job descriptor construction from scheduling so the scheduler becomes a generic job registration service, pushing dataset-kind awareness to the API handler where request context lives.
- Introduce `JobDescriptor` bridge type decoupling the scheduler trait from typed worker crates and metadata-db storage
- Rename `schedule_dataset_sync_job` to `schedule_job` accepting `JobDescriptor`
- Move raw vs. derived job descriptor construction into the deploy handler
- Shift `amp-worker-datasets-{raw,derived}` deps from controller to admin-api
- Remove `SerializeJobDescriptor` error variant from scheduler
Signed-off-by: Lorenzo Delgado <lorenzo@edgeandnode.com>
LNSD
force-pushed
the
lnsd/refactor-admin-api-job-kinds
branch
from
17cc612 to
9936ef2
Compare
2 participants
Separate job descriptor construction from scheduling so the scheduler becomes a generic job registration service, pushing dataset-kind awareness to the API handler where request context lives.
JobDescriptorbridge type, decoupling the scheduler trait from typed worker crates and metadata-db storageschedule_dataset_sync_jobtoschedule_jobacceptingJobDescriptoramp-worker-datasets-{raw,derived}deps from controller to admin-apiSerializeJobDescriptorerror variant from schedulerNote
Medium Risk
Touches the job scheduling interface and how job descriptors are constructed/stored, so mis-conversion or wrong dataset-kind selection could schedule incorrect jobs despite being largely a refactor.
Overview
Refactors job scheduling so the scheduler is a generic job-registration service:
schedule_dataset_sync_jobis replaced byschedule_jobwhich accepts a pre-builtJobDescriptor.Dataset-kind specific descriptor construction (raw vs derived) is moved into the dataset deploy handler, and a new
JobDescriptorbridge type is introduced to convert between typed worker descriptors andmetadata-dbJSON storage (including addingJobDescriptorRaw::into_inner). Dependencies are shifted accordingly (worker descriptor crates move fromcontrollertoadmin-api), and the scheduler error surface is simplified by removing JSON serialization failures.Written by Cursor Bugbot for commit 9936ef2. This will update automatically on new commits. Configure here.