Conversation
kriben
force-pushed
the
multiple-tasks-fan-in
branch
2 times, most recently
from
September 21, 2026 12:08
85d399a to
7456fc8
Compare
kriben
force-pushed
the
multiple-tasks-fan-in
branch
from
September 21, 2026 12:35
7456fc8 to
5f84df5
Compare
kriben
changed the title
There is only one SIGALRM per process, so the previous design where the job, each task, each mapped item, and each nested workflow_task runner called signal.alarm() independently had three defects: 1. Any task/item/inner-runner alarm overwrote the job alarm and its handler; the subsequent signal.alarm(0) then cancelled it for good. A job with timeout_seconds=1 ran to completion in 3s as soon as one task declared its own timeout_seconds. 2. signal.signal() raises ValueError outside the main thread, which was not in the except tuple, and arming happened outside the guarded try block. The exception escaped Runner.run() with job.status left at RUNNING and no TASK_FAIL/JOB_FAIL events. 3. signal.alarm(int(seconds)) truncated: timeout_seconds=1.9 fired at 1s while the error message claimed 1.9s. The job deadline is now an absolute time.monotonic() timestamp carried in a per-run _Deadline object. Before every task and mapped item the deadline is checked and the timer armed with min(own timeout, remaining job time), raising the matching error type. Arming happens inside the guarded region so any failure is recorded as a task failure. ValueError is caught, a single warning is issued per run, and execution proceeds unenforced. Timers use signal.setitimer for float precision, and the pre-existing SIGALRM handler is restored when the run finishes. Regression tests cover: job deadline surviving a task with its own timeout and a nested workflow_task; an expired deadline preventing the next task from starting; a 1.9s timeout allowing a 1.4s task; handler restoration; non-main-thread execution completing with one warning; and an arming failure being recorded as FAILED rather than RUNNING.
ResultPersistenceHook wrote f"{task.name}.json" without escaping, so a
task named "../evil" (from a YAML name: override or a plugin) landed
outside output_dir. Task names are now percent-encoded like map keys
already were, via a shared _safe() helper.
Linear-mode YAML configs (no depends_on/map) were built with the
Workflow(tasks=[...]) shorthand, which ignores YAML name: overrides and
then had config_fields assigned after _validate() had already run. The
result was that per-task input keyed by the overridden name was wired to
a non-existent task, config_fields bypassed type and coverage checks,
and the failure surfaced as an unwrapped WorkflowDefinitionError from
Job(). Both modes now go through WorkflowBuilder, with linear mode
chaining each task on the previous task's registered name, so naming,
config_fields and validation behave identically. Workflow and Job
construction errors are wrapped as ConfigLoadError.
A workflow: entry referencing itself, or two files referencing each
other, recursed until RecursionError. _load_workflow_only now carries
the set of enclosing resolved paths and rejects a cycle with a
ConfigLoadError that names the chain. Reusing an inner workflow at a
different nesting level is still permitted.
…e types
- result_task passed to Workflow() or Workflow.builder() was stored
without checking that it names a registered task, so the failure was a
bare KeyError from .result_task or Runner.run(). _validate_result_task
now raises WorkflowDefinitionError listing the known task names.
- Workflow(tasks=[T, T]) silently overwrote the dict entry and created a
self-edge, which _validate_acyclic then reported as a cycle. Duplicate
names are rejected up front with the same message the builder uses.
_validate_unique_names is documented as satisfied by construction.
- Workflow(tasks=[]) skipped validation entirely because the guard was
truthiness-based. An explicit empty list is now a definition error;
tasks=None remains the "build later" escape hatch.
- Single-dependency and field-reference edges compared types with 'is',
while fan-in and collection edges used _is_type_compatible. A producer
emitting a subclass of the consumer's input model was rejected on the
former and accepted on the latter. All edges now use compatibility.
- Field-reference mismatch messages used .__name__, which drops generic
arguments ("list" instead of "list[int]"). They now go through
_type_name like the other edge kinds.
Hook failures were reported as "Hook X raised during <event>" with the exception discarded, so a broken hook was effectively silent. The warning now includes repr(exc), attaches the exception as the warning's source, and uses a dedicated HookError category (a UserWarning subclass, so existing pytest.warns(UserWarning) and -W filters keep working while callers can now filter or escalate hook failures specifically). A failure inside a workflow_task raised a bare RuntimeError built from result_job.error, losing the exception type, the traceback, and the inner Job with its task_results. It now raises WorkflowTaskError (a TaskExecutionError) carrying the inner Job as .inner_job and chaining the original exception via `from`. To make that chain possible the Runner records the raw exception on Job.exception alongside the existing Job.error string; nested wrappers therefore produce a __cause__ chain that can be walked back to the leaf failure.
_load_workflow_only keyed both task_classes and name_lookup by the import path (or inner workflow file). When the same class or file appeared twice under different name: overrides, the second entry silently overwrote the first, so depends_on: pkg.Cls resolved to whichever instance was declared last, and two workflow: entries for one file could not be wired independently. Entries are now kept positionally, and the lookup maps every key (import path, inner file, instance name) to the set of registered names it denotes. Resolving a key with more than one candidate raises ConfigLoadError listing the candidates and asking for the instance name. result_task goes through the same resolver, so it gets the same not-found and ambiguity handling.
Whether input.yaml was flat root input or per-task configuration was inferred purely from its shape: if every top-level key happened to be a task name with a mapping value, per-task mode was chosen. A flat input whose field name coincided with a task name therefore flipped mode silently and failed with an unrelated coverage error. workflow.input_mode now accepts 'auto' (default), 'flat' or 'per_task'. Explicit modes skip inference; 'per_task' additionally validates that every key is a task name and every value is a mapping or null. Under 'auto' the previous heuristic is retained for the common unambiguous case, but when the mapping would also validate as the sole unconfigured root task's input model the loader raises ConfigLoadError naming both readings and asking for an explicit input_mode.
CI and publish workflows had no permissions block (so the token had the default broad scope), no job timeouts, no concurrency group to cancel superseded runs, and installed dependencies uncached on every run. The third-party publish action was referenced by the mutable release/v1 branch. CI also linted only taskmaestro/ and tests/ although CLAUDE.md lists examples/ among the paths to check. Both workflows now declare `permissions: contents: read` at the top level (publish's second job opts into id-token: write only), set timeout-minutes, and enable setup-python's pip cache. CI gets a concurrency group with cancel-in-progress, lints examples/, and fails if coverage drops below 100%. pypa/gh-action-pypi-publish is pinned to the commit behind v1.14.2. The 3.14 classifier is added to match the test matrix.
This branch has not been deployed
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
1 participant
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.
Fixes #4.
Fixes #8.
Fixes #9.