fix: add jobset and argo events to devtools#3124
Conversation
Greptile SummaryRestores argo-events and jobset support to the Confidence Score: 5/5Safe to merge; all findings are P2 style suggestions that do not affect correctness. Both new components follow the established devtools patterns closely. The two P2 comments (EventBus replicas=3 and curl without checksum) are improvement suggestions for a developer-only environment and do not block merge. devtools/tilt/k8s/argo-events-eventbus.yaml (replicas=3 in a local dev cluster) and devtools/tilt/jobset.tiltfile (remote curl without integrity check). Important Files Changed
Reviews (1): Last reviewed commit: "add argo-events and jobset to pick servi..." | Re-trigger Greptile |
| spec: | ||
| jetstream: | ||
| version: "2.9.15" | ||
| replicas: 3 |
There was a problem hiding this comment.
EventBus replicas=3 is heavy for a dev environment
replicas: 3 launches three JetStream pods, each consuming 100m CPU and 128Mi memory, totalling 300m CPU and ~384Mi. In a typical single-node local cluster (kind/minikube/docker-desktop) this provides no HA benefit since all three pods land on the same node. replicas: 1 is the standard choice for devtools setups and keeps resource consumption in line with the rest of the stack.
| replicas: 3 | |
| replicas: 1 |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
| def setup_jobset(ctx): | ||
| jobset_manifest_url = "https://github.com/kubernetes-sigs/jobset/releases/download/%s/manifests.yaml" % ctx.versions.jobset | ||
| k8s_yaml(local("curl -sSL %s" % jobset_manifest_url)) |
There was a problem hiding this comment.
Remote manifest fetched without integrity check
curl -sSL downloads the jobset manifest from GitHub at every tilt up without verifying its checksum. While the URL is version-pinned, an upstream compromise or MITM between the developer and GitHub would apply arbitrary manifests to the cluster. Consider caching the manifest in-tree (as done with the RBAC file) or at least documenting the known SHA so developers can verify out-of-band.
PR Type
Summary
Recent rework to the metaflow-dev setup removed support for argo-events and jobsets. Add these back.
Issue
Fixes #
Reproduction
Runtime:
Commands to run:
# paste exact commandsWhere evidence shows up:
Before (error / log snippet)
After (evidence that fix works)
Root Cause
Why This Fix Is Correct
Failure Modes Considered
Tests
Non-Goals
AI Tool Usage