Skip to content

docs: deprecate pull_request_analysis_initial_snapshot pipe (IN-1217) - #4480

Open
gaspergrom wants to merge 5 commits into
mainfrom
feat/IN-1217-deprecate-pr-initial-snapshot
Open

docs: deprecate pull_request_analysis_initial_snapshot pipe (IN-1217)#4480
gaspergrom wants to merge 5 commits into
mainfrom
feat/IN-1217-deprecate-pr-initial-snapshot

Conversation

@gaspergrom

Copy link
Copy Markdown
Contributor

Summary

  • Marks pull_request_analysis_initial_snapshot.pipe as DEPRECATED in its DESCRIPTION block and in pull_request_analysis_baseline_merge_MV.pipe's comment — this pipe was replaced by the bucket-pipe architecture (activityRelations_bucket_MV_snapshot_0..9.pipe and activityRelations_collection_bucket_MV_snapshot_0..9.pipe) which handles both bootstrap and ongoing deduplication using consistent hashing (cityHash64(segmentId) % 10 / cityHash64(collectionSlug) % 10, COPY_MODE: append, COPY_SCHEDULE: @on-demand).
  • Updates lambda-architecture.md: marks the pull_request_analysis_initial_snapshot section as deprecated, removes it from the bootstrap CLI usage example, updates the comparison table's status, and adds a concise explanation of the bucket-pipe replacement.
  • No functional/data changes — this pipe is @on-demand only and was never part of any automated flow; this is documentation and pipe-comment cleanup only.

JIRA

IN-1217 — https://linuxfoundation.atlassian.net/browse/IN-1217

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI balanced review requested due to automatic review settings August 15, 2026 02:17
@cursor

cursor Bot commented Aug 15, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Documentation and pipe DESCRIPTION/comment updates only; no runtime or data-pipeline logic changes.

Overview
Clarifies how initial snapshot copy pipes bootstrap the Lambda Architecture in lambda-architecture.md: three independent families (activityRelations per-bucket replace, segment aggregates replace, PR analysis append), when to run each, and operational safety for append-mode PR loads.

PR bootstrap (pull_request_analysis_initial_snapshot) is documented as append + 10-way bucketed (bucket_id / num_buckets=10); the unbucketed tb pipe copy run ... --wait path is marked deprecated (timeouts). The quick-start CLI example drops that single-shot command and adds guidance to empty pull_requests_analyzed before the first bucket and to run all 10 buckets sequentially. The initial-vs-merger comparison table is expanded (pipe names, status, modes, sources/targets).

Pipe metadata/comments: pull_request_analysis_initial_snapshot.pipe DESCRIPTION now states it is the bootstrap/full-rebuild path and points to bucketed invocation; pull_request_analysis_baseline_merge_MV.pipe comments clarify that the MV + hourly merger handle ongoing deduplication after bootstrap, with the initial snapshot still used for first populate (per the doc).

No SQL or schedule changes—on-demand copy behavior is unchanged.

Reviewed by Cursor Bugbot for commit 81a003c. Bugbot is set up for automated code reviews on this repo. Configure here.

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.

Pull request overview

Documents the intended deprecation of the PR-analysis bootstrap pipe in favor of bucket-based processing.

Changes:

  • Marks the initial snapshot pipe as deprecated.
  • Updates related pipeline comments and architecture documentation.
  • Removes the deprecated pipe from bootstrap instructions.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
pull_request_analysis_initial_snapshot.pipe Adds deprecation notice.
pull_request_analysis_baseline_merge_MV.pipe Updates bootstrap-path documentation.
lambda-architecture.md Revises bootstrap guidance and comparison table.

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

Comment thread services/libs/tinybird/pipes/pull_request_analysis_initial_snapshot.pipe Outdated
Comment thread services/libs/tinybird/pipes/pull_request_analysis_baseline_merge_MV.pipe Outdated
Comment thread services/libs/tinybird/lambda-architecture.md Outdated
Comment thread services/libs/tinybird/lambda-architecture.md Outdated
…IN-1217)

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI review requested due to automatic review settings August 15, 2026 02:24

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

services/libs/tinybird/pipes/pull_request_analysis_baseline_merge_MV.pipe:14

  • This MV handles incremental PR-event deltas from activityRelations_enrich_snapshot_MV_ds; the 10-bucket architecture does not bootstrap or deduplicate pull_requests_analyzed. Keep the note scoped to the actual hourly PR path and preserve the historical bootstrap/reset distinction.
    Note: pull_request_analysis_initial_snapshot.pipe (bootstrap) is now DEPRECATED (see lambda-architecture.md);
    the new bucket-pipe architecture handles both bootstrap and ongoing deduplication.

services/libs/tinybird/pipes/pull_request_analysis_initial_snapshot.pipe:2

  • The referenced bucket snapshot pipes only populate activityRelations_bucket_MV_ds_*; they neither compute nor write pull_requests_analyzed. The only other writer, pull_request_analysis_snapshot_merger_copy, depends on an existing pull_requests_analyzed snapshot, so this is not a replacement for historical bootstrap/reset. Please identify the actual replacement path before marking this pipe as replaced.
    **DEPRECATED** — Replaced by the bucket-pipe architecture. See bucketing-architecture.md.

services/libs/tinybird/lambda-architecture.md:591

  • This marks the entire “Initial Snapshot” category deprecated, but the preceding sections and CLI still prescribe the activityRelations and segment aggregate initial snapshots. The struck-through purpose is likewise still valid for those active pipes. Scope this column to the PR initial snapshot or represent the status as mixed.
| **Status** | **DEPRECATED** | Active | Active |
| **Schedule** | @on-demand (manual) | Daily (01:30/01:34/01:38 UTC) | Hourly (0 * * * *) |
| **Mode** | append¹ / replace² | replace (atomic per-bucket swap) | replace |
| **Purpose** | ~~Bootstrap/reset~~ | Incremental merge of MV deltas | Incremental merge of PR events |

services/libs/tinybird/lambda-architecture.md:597

  • This new footnote correctly records COPY_MODE append, but it now contradicts lines 506–510, which state that all initial snapshot pipes use replace. Update the section-level description to distinguish this exception so operators do not receive conflicting bootstrap instructions.
¹ `pull_request_analysis_initial_snapshot`: `COPY_MODE append` (bootstrap by appending)  
² `activityRelations_enrich_initial_snapshot_*` and `segmentId_aggregates_initial_snapshot`: `COPY_MODE replace` (overwrite entire target)

Comment thread services/libs/tinybird/lambda-architecture.md Outdated
…y (IN-1217)

Pipe is not replaced by the hourly PR Merger — that path only handles
ongoing dedup once pull_requests_analyzed is populated. The pipe itself
is still the bootstrap/rebuild mechanism; only its single-shot invocation
is deprecated. Document the bucketed invocation (bucket_id/num_buckets)
that actually replaces it, matching the pipe's own built-in params.

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI review requested due to automatic review settings August 15, 2026 02:51

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

services/libs/tinybird/pipes/pull_request_analysis_initial_snapshot.pipe:5

  • This contradicts the PR's stated scope: the title and summary say this pipe is deprecated and replaced by the 10-bucket snapshots, but this new description declares it active and required for rebuilds. Those bucket snapshots target activityRelations_* datasources rather than pull_requests_analyzed, so the PR metadata/IN-1217 intent and these changes describe different architectures. Please align them before merging.
    Bootstrap/full-rebuild pipe for pull_requests_analyzed. Do not run single-shot
    (`--wait` with no bucket params) — it does not finish against the full dataset.
    Run bucketed instead, one bucket_id at a time (see lambda-architecture.md):
    tb pipe copy run pull_request_analysis_initial_snapshot --param bucket_id=N --param num_buckets=10 --mode append

services/libs/tinybird/lambda-architecture.md:544

  • Without --wait, the CLI returns after starting each copy job, so this loop launches all ten expensive historical scans instead of processing one bucket_id at a time. That can recreate the overload this bucketing is intended to avoid and lets the command finish before the bootstrap completes. Serialize the jobs and surface failures by waiting for each run.
  tb pipe copy run pull_request_analysis_initial_snapshot --param bucket_id=$N --param num_buckets=10 --mode append

services/libs/tinybird/lambda-architecture.md:548

  • The named snapshot family does not use this runtime subbucketing mechanism: each activityRelations_bucket_MV_snapshot_N.pipe hardcodes cityHash64(segmentId) % 10 = N and has no bucket_id/num_buckets parameters. Equating the two conflates ten fixed top-level pipes with parameterized chunks and gives readers an incorrect architecture model.
This is the same subbucketing mechanism used by the generic `activityRelations_bucket_MV_snapshot_*` family described in `bucketing-architecture.md`, applied here directly inside `pull_request_analysis_initial_snapshot.pipe` rather than via a separate set of pipe files.

Comment thread services/libs/tinybird/lambda-architecture.md Outdated
…IN-1217)

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI review requested due to automatic review settings August 15, 2026 03:08

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

…tally committed (IN-1217)

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI review requested due to automatic review settings August 15, 2026 03:14

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (5)

services/libs/tinybird/lambda-architecture.md:550

  • This command is labeled correct but is not valid shell: the for loop omits do/done, and ... is not a pipe name. Use the complete runnable command so operators do not copy a bootstrap procedure that fails immediately.
  ✓ CORRECT:   for N in $(seq 0 9); tb pipe copy run ... --param bucket_id=$N --param num_buckets=10

services/libs/tinybird/lambda-architecture.md:569

  • This source description does not match the pipe being documented. segmentId_aggregates_initial_snapshot.pipe:10-12 still queries activityRelations_deduplicated_cleaned_ds, and no datasource definition for that removed name exists in the repository. Updating only this prose to the bucket union makes the documented command appear functional while its implementation still uses the stale datasource; update the pipe source as part of this change or document that this bootstrap is currently unavailable.
├─ Queries activityRelations_enriched_deduplicated_bucket_union at latest snapshot

services/libs/tinybird/pipes/pull_request_analysis_initial_snapshot.pipe:3

  • The PR title and description still say this pipe itself is deprecated and replaced by the 10-bucket activityRelations pipes, but this description—and lambda-architecture.md:623—now establish that it remains the required pull_requests_analyzed bootstrap and only the unbucketed invocation is deprecated. Please update the PR metadata to match the implemented scope, or restore the stated deprecation with an equivalent bootstrap replacement.
    Bootstrap/full-rebuild pipe for pull_requests_analyzed. Do not run single-shot
    (`--wait` with no bucket params) — it does not finish against the full dataset.

services/libs/tinybird/lambda-architecture.md:601

  • The loop is described as sequential, but without --wait each CLI invocation returns after starting its copy, so the next bucket can be submitted while the previous append to the same datasource is still running. The repository's copy runner uses --wait for this reason (scripts/run_activity_merge_copy_pipe.sh:75-80). Wait for each bucket to complete so failures stop the procedure instead of leaving a partial bootstrap.
  tb pipe copy run pull_request_analysis_initial_snapshot --param bucket_id=$N --param num_buckets=10 --mode append

services/libs/tinybird/lambda-architecture.md:605

  • The PR merger cannot repair duplicates from an interrupted/retried bootstrap. Its historical_snapshot node copies every existing row whose key is not in that hour's realtime delta (pull_request_analysis_snapshot_merger_copy.pipe:8-24), so duplicate untouched PRs survive the replace. Require clearing the target and restarting all buckets rather than offering the merger as a recovery path.
If you interrupted a mid-run or suspect duplicates exist, either clear the datasource before retrying or use the scheduled PR Merger (`pull_request_analysis_snapshot_merger_copy`) to replace the entire snapshot (but note it requires the existing baseline to compute deltas — an empty datasource will produce no output).

@gaspergrom
gaspergrom requested a review from epipav August 15, 2026 03:18
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