Skip to content

feat(odin): Add intermediate progress reporting and report_progress API to ActionContext#548

Draft
vikramlc-cognite wants to merge 4 commits into
EDG-371-upload-logs-to-cdf-filesfrom
EDG-372-build-action-result-progress
Draft

feat(odin): Add intermediate progress reporting and report_progress API to ActionContext#548
vikramlc-cognite wants to merge 4 commits into
EDG-371-upload-logs-to-cdf-filesfrom
EDG-372-build-action-result-progress

Conversation

@vikramlc-cognite

Copy link
Copy Markdown
Contributor

Summary

Adds ActionContext.report_progress() as a clean API for mid-action progress updates, and wires it into the fetch_logs upload loop so Odin receives a running status update with "Uploading: N/M files complete" after each file resolves.

Type of change

  • New feature (non-breaking change that adds functionality)

What changed

  • actions.py: Added ActionContext.report_progress(message) - queues a running ActionUpdate with the given message via _checkin_worker, imported ActionStatus/ActionUpdate from _dto at module level
  • _log_upload_action.py: Replaced the one-liner upload_results.extend(... for future in as_completed(...)) with an explicit loop; after each future resolves, calls ctx.report_progress(f"Uploading: {completed}/{total} files complete")
  • test_log_upload_action.py: 4 new tests - report_progress unit test (queues correct external_id, status, message), multi-file progress (2 files → both 1/2 and 2/2 messages present), single-file progress (1/1), and no-candidates case (zero progress updates, still succeeds)

Why it changed

  • The upload loop previously collected all results silently then reported a single final succeeded. For long-running uploads (e.g. 7 × 1 GB files at 10 Mbps each), callers had no visibility into whether the action was progressing or stalled.

What to focus on during review

  • report_progress denominator: Uses len(candidates) (files actually being uploaded), not num_days (requested date range). A 3-day request where 1 file is missing shows 1/2, 2/2 - not 1/3, 2/3. This is intentional: the denominator tracks upload progress, not date coverage. The final succeeded message already uses num_days for the overall picture.
  • No progress when no candidates: If all files are missing/skipped, the for loop body never executes - no spurious progress updates are emitted before succeeded.

Test evidence

tests/test_unstable/test_log_upload_action.py 41 passed (4 new)
ruff check + ruff format - all clean

Risks and unknowns

  • report_progress is now part of the ActionContext public API - user-defined actions can also call it. The method is intentionally general; naming it with an upload-specific prefix was avoided for reusability.

Rollout and rollback

  • No config changes or migrations. Rolling back removes the intermediate running updates; Odin handles a running → succeeded transition without intermediate updates, so rollback is safe.

Checklist

  • Self-reviewed the diff
  • Tests added (4 new tests covering unit, multi-file, single-file, and empty-candidate cases)
  • Docs N/A
  • No secrets, credentials, or PII committed
  • No breaking changes

@vikramlc-cognite vikramlc-cognite self-assigned this Jun 29, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces intermediate progress reporting for log upload actions by adding a report_progress method to ActionContext and reporting progress as files complete uploading. Feedback was provided to throttle these progress updates (e.g., reporting at most 10 times) to prevent queue bloat and artificial delays when uploading a large number of files.

Comment thread cognite/extractorutils/unstable/core/_log_upload_action.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants