Skip to content

fix(pipelines): read code blobs from raw URLs and fail the crawl on unreadable paths - #246

Open
Neilblaze wants to merge 1 commit into
kubeflow:mainfrom
Neilblaze:fix/code-pipeline-silent-empty-index
Open

fix(pipelines): read code blobs from raw URLs and fail the crawl on unreadable paths#246
Neilblaze wants to merge 1 commit into
kubeflow:mainfrom
Neilblaze:fix/code-pipeline-silent-empty-index

Conversation

@Neilblaze

Copy link
Copy Markdown

Fixes #245

Why

code-pipeline.py feeds code_rag, the collection behind search_kubeflow_code. Three things make it index far less than it reports, none of which fail the run.

Three of four default directories 404. kubeflow/manifests moved apps/ to applications/, so apps/pipeline/upstream, apps/katib and apps/jupyter are gone. api_request returns None on a non-200, get_files_recursive turns that into [], and the component prints Found 0 files and exits 0. Only common/istio still resolves: 84 files against 570 for the corrected paths. The same path swallows 5xx and exhausted rate limits.

Files over 1 MiB are stored empty. The Contents API only base64-encodes blobs under 1 MiB; above that it answers 200 with "encoding": "none" and an empty content. The "content" in file_resp guard passes, b64decode("") yields b"", and the file is dropped downstream as "tiny". Five YAML files in the repo are affected, 22.1 MB, all KServe/Spark/Ray install manifests.

Embedding responses are not length-checked, unlike utils.embed_texts. A short batch leaves records with no embedding, which fails one component later as a bare KeyError.

What

  • Read blobs from the listing's download_url. Directory listings already carry download_url and html_url for every entry, including ones over the cap, and the listing's html_url is identical to the per-file one, so citations are unchanged. This drops the per-file Contents API call: for the default directories the REST requests go from 780 to 210. The 570 blob reads move to raw.githubusercontent.com, which costs no quota at all (measured: core.used is unchanged across raw fetches). Note the old repo name redirects, and GitHub bills the redirect too, so those 210 requests still cost 420 of the 5,000/hour budget rather than 210 -- down from 1,560.
  • Fail on a request the crawl cannot recover from. request_with_retries still waits out rate limits and now retries 5xx (which the old code never did), but raises once the retries are spent instead of returning None, so a directory or file it cannot read ends the run. Rate-limit detection now covers 429 and Retry-After, not just 403 with a zeroed X-RateLimit-Remaining — GitHub uses both, and a secondary limit only sets Retry-After. A directory that exists but matches no extensions prints a warning rather than passing unremarked.
  • Refresh the defaults to applications/pipeline,applications/katib,applications/notebooks-v1,common/istio. applications/notebooks-v1 is the direct rename target of apps/jupyter; applications/workspaces is Notebooks 2.0, a separate tree, and is left out. Dropping the /upstream suffix on the pipeline path adds three overlay files (267 vs 264).
  • Restore the embedding length check that utils.embed_texts already applies.
  • Two stale references to the old default in tests/eval/ updated to match.

repos still says kubeflow/manifests, which redirects to kubeflow/community-distribution. Switching to the canonical name would halve the quota cost, but it would also change file_unique_id on every record and orphan the existing rows, since store_code_milvus deletes by that key. That trade is a separate call from this fix, so the name is left alone.

Verification

Against main all four new tests fail, each on the symptom described above:

assert '' == 'apiVersion: ...'                     over-cap blob stored empty
Failed: DID NOT RAISE <class 'RuntimeError'>       missing directory swallowed
Failed: DID NOT RAISE <class 'RuntimeError'>       short embeddings batch accepted
assert [] == [7]                                   429 + Retry-After not waited out

Directory and size facts checked live against kubeflow/manifests@master; the 570-vs-84 file counts come from the Git tree API with the pipeline's own extension filter.


Notes for reviewers

Where the new tests run. All four need the KFP SDK. PR Safety installs only requirements-test.txt, so it skips them (137 passed / 9 skipped, against 137 / 6 on main). Build, Test, and Deploy to OKE installs kfp kfp-kubernetes and runs on every pull request, so they do execute there.

Size the pod before crawling KServe. These manifests were previously dropped, so reading them is a real increase: kserve.yaml alone becomes 7,872 chunks and 984 TEI batches at the default chunk_size=1000 / embedding_batch_size=8. No component in pipelines/ sets a memory limit.

Behaviour change worth naming. The per-file Contents API call used to resolve symlinks; reading the raw URL stores the link target path instead. kubeflow/manifests contains no symlinks or submodules, so nothing in the default crawl is affected.

Likely conflicts. No open PR edits code-pipeline.py in place, but #231 moves docs-agent-mcp/ to kagent-feast mcp/ and rewrites tests/test_code_utils.py wholesale, and #205/#206 add both files at the pre-move pipelines/ path.

Not addressed here

The same missing embedding check exists in kubeflow-pipeline.py, issues-pipeline.py and incremental-pipeline.py; #212 owns the issues pipeline and #244 the incremental one, but kubeflow-pipeline.py is unowned and worth a follow-up. chunk_and_embed_code also still hardcodes max_tei_chars = 1000 while the docs pipeline made it a parameter in #234 — the same kind of change a reviewer asked for there, and a reasonable follow-up once #212 settles the shared value.

…nreadable paths

Signed-off-by: neilblaze <putubanerjee23@gmail.com>
@google-oss-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tarekabouzeid for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(pipelines): three of four default code-crawl directories 404 and the run still succeeds

1 participant