job_runs: add lifecycle.triggers.on_file_change - #6309
Conversation
Integration test reportCommit: 47994d8
8 interesting tests: 4 SKIP, 3 RECOVERED, 1 KNOWN
Top 6 slowest tests (at least 2 minutes):
|
Approval status: pending
|
b8c20f6 to
8744e5e
Compare
Re-fire a run when matched file contents change, using stable hash fingerprints so mtime-only updates (touch) do not recreate.
Satisfy the linter on PrepareState literals after adding OnFileChange, and refresh out.fields.txt so validate-generated stays green.
Reject patterns outside the sync root and directory-only matches so the trigger cannot fingerprint escaped paths or silently disarm.
Drop size/mtime fingerprints and the resolve fast path so content identity alone drives recreate, and touch no longer needs a planner exception.
The modernize linter rejects the manual m[k]=v copy loop, failing CI lint.
8744e5e to
47994d8
Compare
| job_id: ${resources.jobs.my_job.id} | ||
| lifecycle: | ||
| triggers: | ||
| - on_file_change: seed.txt |
There was a problem hiding this comment.
do you now have a test for glob? That test should check what happens if one of the file under glob changed or glob set itself changed
Also please add a test for ** even if it does not work, to see that the error is clear.
Also, add an ignored file there -- glob should respect ignores the same way 'sync' does.
There was a problem hiding this comment.
Also test something in subdirectory mysubdir/.txt
and in outer directory ../shared/.txt
Should also test what happens if global matches directories: */*.py
|
|
||
| === editing the file re-fires | ||
| >>> [CLI] bundle plan | ||
| recreate job_runs.my_run |
There was a problem hiding this comment.
Can you include json plan here instead? Also add READPLAN variant.
| trace $CLI bundle deploy | ||
| read_id.py my_run > /dev/null | ||
| trace print_requests.py --keep //jobs/runs/delete | ||
| trace print_requests.py //jobs/run-now |
There was a problem hiding this comment.
What about removing the file? and adding it back?
| out[filepath.ToSlash(pattern)] = missingFileHash | ||
| return out, diags.Append(diag.Diagnostic{ | ||
| Severity: diag.Warning, | ||
| Summary: fmt.Sprintf("lifecycle.triggers.on_file_change: no files match %q", pattern), |
There was a problem hiding this comment.
This and other errors should have an acceptance test.
| } | ||
| defer f.Close() | ||
|
|
||
| h := sha256.New() |
There was a problem hiding this comment.
This implementation is fine but it also does way too much potentially.
We could also do this:
- store stat(x).st_size and stat(x).st_mtime in the state in addition to hash
- check stat values first:
= st_size is different? can trigger update without calculating the hash
= st_size the same and t_mtime is the same? can skip hashing, file was not touched so hash can be assumed the same.
However, let's discuss this first, because we do something similar in file upload so I wonder if we should just re-use the mechanism there.
There was a problem hiding this comment.
I tried size+mtime in state in an earlier revision. touch then showed up as local drift, so we needed ignore_local_changes on mtime_ns to prevent it from recreating. Since mtime isn’t part of the recreate condition, putting it in planned state felt less elegant than just hashing the content. I also wouldn’t reuse file upload for this: sync is mtime-only, so a touch would trigger a re-upload and re-fire the job. What do you think?
There was a problem hiding this comment.
right, let's keep your implementation it's simple and you only pay for files you track. We can add local optimization later, maybe using cache that we have.
Changes
Add
lifecycle.triggers.on_file_changeforresources.job_runs(direct engine).touchdoes not re-fire).prevent_destroy.Why
Users need to re-run a job when local inputs change (SQL migrations, notebooks, configs) without re-firing on every deploy. Content hashes keep the trigger stable under sync noise while still catching real edits.
Tests
PrepareState/OverrideChangeDesc.acceptance/bundle/resources/job_runs/on_file_change— deploy, unchanged redeploy, touch, edit → recreate, clear trigger → update-only.