Skip to content

Read processing status from the v2 telemetry Worker - #172

Open
seandavi wants to merge 1 commit into
masterfrom
telemetry-v2
Open

seandavi wants to merge 1 commit into
masterfrom
telemetry-v2

Conversation

@seandavi

@seandavi seandavi commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Processing telemetry moved from the retired v1 server (nf-telemetry.cancerdatasci.org / the Cloud Run URL) to the v2 Cloudflare Worker, https://nf-telemetry.seandavi.workers.dev.

Changes

  • inst/scripts/build_manifest.py: TELEMETRY now points at v2, and requests send User-Agent: curatedMetagenomicDataCuration-manifest/1 (Cloudflare returns 403 to urllib's default agent). v2's /api/samples items still carry ncbi_accession, metadata and collections, so the indexing code is unchanged. --offline and the cache are untouched.
  • R/nf_get_completed_run.R (+ man/): repointed from the dead Cloud Run URL to /api/runs?status=completed&limit=<limit>. Returns the runs data frame (run_name, run_id, workflow_id, workflow_version, completed_at, plus other run fields). There is no per-run sample list any more.

Expected drop in processed counts

v2 currently holds only a demo set (5 collections, 74 samples, 4 completed jobs). Regenerating the manifest locally gave n_samples_processed = 74, versus 1,727 in the committed CSV. This is expected: v1 history is retired and the corpus is being re-registered (nextflow_telemetry #196). Counts will climb back as that proceeds.

The regenerated studies_status.csv / studies_runs.tsv are not in this PR. update-manifest.yml rebuilds them on push to master, since this PR touches build_manifest.py.

Caveats: v2 sample items have an empty metadata, so telemetry no longer supplies BioProjects (the script falls back to SRA metadata and NCBI). And /api/samples lists registered samples, not necessarily completed ones.

Refs nextflow_telemetry #193, #194, #196.

Telemetry moved from the retired v1 server to the v2 Cloudflare Worker
(https://nf-telemetry.seandavi.workers.dev).

- build_manifest.py: point TELEMETRY at v2 and send a User-Agent header
  (Cloudflare 403s urllib's default). v2's /api/samples item shape
  (ncbi_accession, collections, metadata) is unchanged, so no other code
  changes were needed.
- nf_get_completed_run(): repoint at /api/runs?status=completed and document
  v2's run fields (run_name, run_id, workflow_id, workflow_version,
  completed_at); the response carries no per-run sample list.

Expected drop: v2 holds only a demo set (5 collections, 74 samples), so
n_samples_processed falls from 1,727 to 74 when the manifest regenerates.
v1 history is retired and the corpus is being re-registered
(nextflow_telemetry #196). The regenerated CSV/TSV are not committed;
update-manifest.yml rebuilds them on push to master.

Refs nextflow_telemetry #193, #194, #196.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Metadata Validation Report

Date: $(date -u +"%Y-%m-%d %H:%M:%S UTC")
Trigger: pull_request
Branch: 172/merge

Schema Information

  • Repository: shbrief/OmicsMLRepoCuration
  • Commit: c5f2be0

Results

  • Total Files: 150
  • Status: ❌ FAIL

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The manifest currently counts all registered v2 samples as processed, including samples without completed jobs.

1 open finding
What changed in this PR

Migrates processing telemetry clients from the retired v1 service to the v2 Cloudflare Worker.

Changes:

  • Updates manifest telemetry endpoint and User-Agent.
  • Updates completed-run retrieval and documentation for v2 responses.
  • Returns the v2 API’s runs data frame.
File Description
R/​nf_get_completed_run.R Uses the v2 completed-runs endpoint.
man/​nf_get_completed_run.Rd Documents the new response shape.
inst/​scripts/​build_manifest.py Reads v2 sample registrations with a Cloudflare-compatible User-Agent.
Files not reviewed (1)
  • man/nf_get_completed_run.Rd: Generated file

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@@ -160,7 +168,8 @@ def fetch_telemetry_samples(offline):
out, offset = [], 0
while True:
url = f"{TELEMETRY}/api/samples?limit=500&offset={offset}"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Registered samples are incorrectly treated as completed processing results, producing inaccurate manifest counts and statuses.

1 open finding
Files not reviewed (1)
  • man/nf_get_completed_run.Rd: Generated file

🧠 Review effort: Balanced

@lwaldron

lwaldron commented Oct 8, 2026

Copy link
Copy Markdown
Member

I verified this against the live Worker: urllib's default agent really does get a 403 and the custom agent gets 200, both endpoints return the documented shapes, and nf_get_completed_run() returns a proper data frame (7 rows just now). The URL and User-Agent changes are all fine.

I agree with Copilot's finding though, and I'd rather fix it here than as a follow-up, because merging triggers update-manifest.yml and commits the regenerated manifest right away. /api/samples is registrations, so n_samples_processed would claim ~137 processed when only a handful of jobs have completed, and it would stay wrong for as long as re-registration outpaces processing. The endpoint Copilot suggests works and has what index_telemetry() needs: /api/workflows/{pk}/jobs?status=completed returns {items, after, limit} (cursor pagination on after), and each item carries ncbi_accession and collections. Iterating /api/workflows for the pks and paging each one's completed jobs looks like a small change to fetch_telemetry_samples().

Everything else checked out, so I'll merge as soon as that's in.

🤖 Verified with Claude Code

@lwaldron

lwaldron commented Oct 8, 2026

Copy link
Copy Markdown
Member

I'll let you decide @seandavi whether to add to this PR or just have me merge as-is. Seems like a bigger deal now while there is a bigger lag between registration and completion of jobs, and less when completion catches up.

This branch has not been deployed

No deployments
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.

3 participants