Add Task execution and failure introspection (v0.8.0)#514
Open
dotsdl wants to merge 9 commits into
Open
Conversation
Implements the v0.8.0 introspection design (milestone 9): durable execution provenance, live progress reporting, failure introspection (tracebacks, per-unit logs, stdout/stderr), and compute share reporting. Addresses #106, #211, #347, #195, #415, #295, #349, #389. - Durable provenance: an immutable TaskProvenance record per execution attempt (created at claim, finalized at result/expiry/deregistration/ release, race-safe against late results), hostname on compute service registrations, and Task.datetime_status_changed / Task.reason indicators with centralized status writes across every mutation site. - Live progress: an alchemiscale-owned execute_DAG mirroring gufe's semantics plus unit-attempt hooks (guarded by a gufe-equivalence suite), driving event-driven, fire-and-forget progress reporting. - Failure introspection: fast bulk tracebacks; per-unit log capture over the gufekey logger namespace plus gufe-native stdout/stderr capture; ProtocolUnitResultRef nodes and artifact retrieval; and graceful ProtocolDAG creation-failure handling (Task -> error with reason rather than killing the compute service). - Compute share: get_scope_compute_share. - Client methods, API routes, ComputeServiceSettings, a v04_to_v05 index migration, Sphinx docs, news fragments, and unit + integration tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- compute API set_task_result: tolerate a missing/`"None"` compute_service_id (the client serializes an absent id as the string "None"); skip provenance finalization instead of raising ValueError -> 500. Fixes the existing test_set_task_result / test_set_task_result_failure integration tests. - equivalence suite: pass `mapping=None` to every `Protocol.create()` call; the pinned gufe requires `mapping` as a keyword-only argument with no default (a newer local gufe had masked this). - docs: replace the unresolvable `gufe...ProtocolUnit.logger` intersphinx attr references with plain literals, so the docs build (fail_on_warning) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #514 +/- ##
==========================================
+ Coverage 80.83% 80.98% +0.14%
==========================================
Files 31 33 +2
Lines 4916 5716 +800
==========================================
+ Hits 3974 4629 +655
- Misses 942 1087 +145 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The Sphinx build (fail_on_warning) flagged autodoc'd docstrings with "inline interpreted text ... start-string without end-string": napoleon's field-list rendering surfaces reStructuredText's markup rules, which reject a closing backtick immediately followed by a letter/apostrophe (`Task`s, `Task`'s) and slash-joined inline markup (`_scoped_key`/`_gufe_key`). Reworded the affected docstrings (get_tasks_details, get_tasks_progress, get_task_tracebacks, get_task_stdout/stderr, get_scope_compute_share, ProtocolUnitResultRef) to keep the code markup while satisfying the markup rules. Verified zero inline-markup warnings across all changed docstrings by running each through napoleon + docutils. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The RTD Sphinx build (older/stricter than local) still flagged TaskProvenance:49 --- backtick-plural (`ProtocolUnit`s) inside a napoleon Attributes section, a context my local docutils reproduction couldn't see (the `.. attribute::` directive masked the inline content). Reworded that and, defensively, the other backtick-plural/apostrophe occurrences in the new docstrings so the render is robust to the stricter build. Pre-existing methods (get_task_results etc.) are untouched; they use the same style in plain summary prose and build cleanly on main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- tests/unit/test_introspection_records.py: to_dict/from_dict round-trips (including None branches) for TaskProvenance and the client-facing record models (TaskAttempt, TaskClaim, TaskDetails, TaskTracebacks, TaskUnitTraceback, ProtocolDAGResultRec, ProtocolUnitResultRec), plus the datetime coercion helpers and the ProtocolUnitResultRef gufe round-trip. - tests/integration/storage/test_objectstore_introspection.py: push/pull round-trips for the per-unit log and stdout/stderr artifact methods (moto-mocked), including UTF-8 errors="replace" decoding, empty-prefix pulls, and invalid-stream-name validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
End-to-end (client -> API -> Neo4j/S3) coverage for the new user-facing methods: get_task_history, get_tasks_details, get_task_result_recs, get_result_unit_recs, get_result_unit_logs/stdout/stderr, get_result_logs, get_task_stdout/stderr, get_task_tracebacks, get_tasks_progress, get_scope_compute_share, and set_tasks_status(reason=...). State (claimed provenance, results, per-unit refs, and per-unit artifacts) is set up via the n4js/s3os handles as the existing result tests do, then each method is called through the real client and asserted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_scope_compute_share at the org level requires org-level scope access; the test identity holds only the specific project scope, so the org-level query 401'd (correct authorization behavior). Query at the project scope the identity holds instead; the org-level branch is already covered by the statestore integration test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…are levels) Nudge patch coverage over the threshold by exercising the last uncovered branches in already-passing tests: - get_result_logs(order='time') interleave path (with a timestamped and an untimestamped log line, hitting both sort keys); - get_scope_compute_share campaign-level and project-level grouping, plus the no-org ValueError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ire tests
- H1 (HIGH): the /computeservice/{id}/progress route declared Body(embed=True)
while the compute client sends the bare map, so every live-progress push
422'd (silently, swallowed at DEBUG) and the whole feature returned None in
production. Drop embed so the route accepts the bare map per the design's
transport spec; also skip malformed entries instead of 500'ing.
- M2: add end-to-end compute-client tests through the API for
update_task_progress (would have caught H1), set_task_error (the /error
DAG-creation-failure path), and set_task_result_unit_logs (the artifact
upload route).
- M1: rename the migration v04_to_v05 -> v07_to_v08. Migration names track
release versions (v03_to_v04 shipped in v0.4.0); this is the v0.7 -> v0.8
upgrade, so as named an operator upgrading 0.7->0.8 would never run it and
the TaskProvenance indexes would never be created. Update the CLI command
and document the migration in docs/operations.rst.
- N4: constrain the `limit` query param on /history and /tracebacks to ge=1
(422 instead of a 500 from a negative Cypher LIMIT).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
Implements the accepted v0.8.0 Task execution and failure introspection design (milestone 9). The goal is to let users understand what is happening with their
Tasks: where and how long they've run, how far along they are, why they failed, and how much compute aScopeis getting.Addresses #106, #211, #347, #195, #415, #295, #349, #389.
What's included
1. Durable execution provenance (#106, #211)
TaskProvenancerecord per execution attempt, created at claim time and finalized when the attempt ends (complete/error/expired/released). It survives claim teardown and registration expiry, so "how many times has thisTaskrun, and where?" is answerable.(task, compute_service_id), a late result overwrites only its own prematurely-expiredrecord and never touches another service's open one.hostnameonComputeServiceRegistration(copied onto every provenance record); newTask.datetime_status_changedandTask.reasonindicators, written through a single centralized helper at every status-mutation site (claim,set_task_*, expiry, deregistration, restart-renew).get_task_history,get_tasks_details.2. Live progress reporting (#415)
execute_DAG(alchemiscale.compute.execute) mirroring gufe's execution semantics plus unit-attempt start/end hooks — the single seam progress and log capture plug into. A behavioral-equivalence test suite runs identical DAGs through both implementations and asserts equivalentProtocolDAGResults.TaskProvenance, so they survive claim teardown.get_tasks_progress; progress also appears in history/details.3. Failure introspection (#347, #349, #295, #195)
get_task_tracebacks).gufekeylogger namespace, plus gufe-native per-unitstdout/stderrcapture;ProtocolUnitResultRefnodes (withCONTAINS/UNIT_DEPENDS_ONtopology) and object-store artifacts.get_task_result_recs→get_result_unit_recs→get_result_unit_logs/get_result_unit_stdout/get_result_unit_stderr, plus renderingsget_result_logs,get_task_stdout/get_task_stderr.ProtocolDAGcreation-failure handling: theTaskis set toerrorwith the traceback inTask.reasonand the service continues, instead of the failure killing the loop and silently bouncing Tasks back towaiting.4. Compute share reporting (#389)
get_scope_compute_share(scope)— the fraction of currently-runningTasks aScopeconsumes relative to its same-level siblings (only the aggregate is disclosed).Also: new
ComputeServiceSettings(hostname,capture_streams,capture_logs,gufekey_loglevel,log_cap_bytes,progress_push_timeout), av04_to_v05index migration (idempotent, no data migration required), Sphinx docs (handling_errors.rst, a newintrospection.rst, compute settings), andnews/fragments.Data model changes
All additions are optional-valued, so no data migration is required — pre-existing
Tasks simply have empty history andnullindicators.TaskProvenancenode +PROVENANCE_OFedgesProtocolUnitResultRefnode +CONTAINS/UNIT_DEPENDS_ONedgesTask.datetime_status_changed,Task.reasonComputeServiceRegistration.hostnameTesting
tests/unit/compute/): a gufe-equivalence suite for the executor (success / failure / retries / retry-then-success / interrupts / cache-resume / stream-dir parity / hooks) proving byte-for-byte behavioral parity with the pinned gufeexecute_DAG, plus log-capture unit tests.tests/integration/storage/test_statestore_introspection.py, 22 tests) cover the provenance lifecycle, the late-result overwrite rules, status/reason indicators (including the idempotent-no-op guard), history/details/tracebacks/progress, unit refs, and compute share. These require the Neo4j/grolt harness and will run in CI.Notes for reviewers
datetime_status_changed/reasonagainst no-op re-sets using a CypherCASEthat reads the pre-clause status.test_status_write_idempotent_noop_preserves_indicatorsvalidates this against real Neo4j._pu_to_pur/_get_valid_unit_resultsto minimize divergence; the equivalence suite is the guard to run on every gufe upgrade.Tasksystem to retainProtocolDAG, uploadProtocolUnitResults andResultFiles as they complete #180; several choices (unit-attempt hooks,ProtocolUnitResultRefshape, always recordinglocation+ recompute-first/location-fallback retrieval) were made to carry over cheaply.🤖 Generated with Claude Code