Repository navigation
Conversation
This comment was marked as spam.
This comment was marked as spam.
Pedrinhonitz
suggested changes
Jun 1, 2026
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
uranusjr
reviewed
Jul 31, 2026
cursor
Bot
force-pushed
the
fix/62929-use-job-schedule-asset-runs
branch
from
August 19, 2026 01:14
486cb0f to
900e7ab
Compare
Pedrinhonitz
approved these changes
Aug 19, 2026
Pedrinhonitz
left a comment
Contributor
There was a problem hiding this comment.
That works for me; the process can proceed to the maintainer's review, where the impact will be assessed more precisely.
[scheduler] use_job_schedule is documented to turn off only cron/time-based scheduling, but it gated the entire _create_dagruns_for_dags call, which also creates asset- and partitioned-asset-triggered runs. As a result, setting the flag to False silently stopped all asset-driven scheduling. Move the flag check so it gates only time-based run creation; asset- and partition-triggered runs are now always created, matching the documented behavior. Also update the "Disable the scheduler" maintenance docs, which previously claimed this flag stops all new Dag runs. closes: apache#62929
Align the asset-triggered setup with current catchup/backlog gating (apache#39456): create the consumer before events and place AssetEvent / AssetDagRunQueue timestamps inside the consumer's schedule window so consumed_asset_events is populated under catchup=False.
The same rationale was stated verbatim at both call sites, which review feedback flagged as too verbose. Keep one concise note at the actual gate.
AssetDagRunQueue now requires asset_event_id, so the use_job_schedule regression test was failing Serialization DB CI with a NOT NULL IntegrityError instead of exercising the scheduler flag.
AssetDagRunQueue now requires asset_event_id, so the use_job_schedule regression must persist the event before queueing or Serialization CI fails with a NOT NULL IntegrityError instead of testing the flag.
Fork workflows were waiting on approval after a bot push.
cursor
Bot
force-pushed
the
fix/62929-use-job-schedule-asset-runs
branch
from
August 24, 2026 00:19
7888fec to
f66a5e9
Compare
1 task done
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.
What is the change?
[scheduler] use_job_schedule=Falsenow disables only time-based (cron/timetable) scheduling. Asset-triggered and partitioned-asset-triggered Dag runs are still created, as are manual runs.Why did I do it?
The flag gated the entire
_create_dagruns_for_dagscall in_do_scheduling, but that method also creates asset-triggered and partitioned-asset runs. Setting the flag toFalsesilently stopped all asset-driven scheduling, contradicting the documented behavior. This supersedes #62931, which was closed for author inactivity; the approach itself was never disputed.closes: #62929
How did I do it?
I moved the flag check inside
_create_dagruns_for_dagsso it wraps only the time-based_create_dag_runs(non_asset_dags, ...)call. The orchestrator, and with it the asset and partition paths, always runs. I also fixed the "Disable the scheduler" maintenance doc, which claimed the flag stops all new Dag runs, and pointed readers atdags pausefor a full stop.What's the impact?
Deployments that turn off cron scheduling keep their event- and data-driven pipelines. No behavior change with the flag at its default
True.What's the test plan?
Three regression tests in
test_scheduler_job.py, each verified to fail on the unfixed code: an asset-triggered run created with the flag off, a partitioned-asset run created with the flag off, and a flag-on companion proving cron runs are still gated. The full scheduler suite, the exact-query-count tests, mypy, and the static checks passed.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.8) following the guidelines