docs: update README for OCI with sidecar#97
Merged
julpayne merged 2 commits intoeval-hub:mainfrom Mar 27, 2026
Merged
Conversation
now OCI Artifact Push goes through sidecar for Authentication Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Contributor
📝 WalkthroughWalkthroughUpdated README.md: the architecture diagram now inserts a dedicated sidecar step (S3) between the adapter callback and the OCI Registry, and documentation/code snippets were revised to use adapter-driven callback construction and adapter job spec for benchmarking; removed direct persister examples. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 37: Update the S3 node label S3["Authenticated Push OCI artifacts
to<br/>OCI Registry"] to a clearer, consistent noun- or verb-phrase (e.g.,
"Authenticated OCI Artifact Push" or "Push OCI Artifacts (Authenticated)") so
the diagram reads cleanly; replace the current awkward sentence with one of
these concise label forms and preserve the HTML line break if needed for layout.
- Around line 43-48: The architecture diagram now shows OCI artifact pushes
routed via the sidecar (nodes A4 -> S3, A5 -> S4 and edges S2 -> EvalHub, S3 ->
Registry), but the README prose in the "Key Points" section still states the SDK
pushes artifacts directly; update that prose and any examples to say the SDK
performs pushes through the sidecar sidecar component (EvalHub/sidecar) to the
OCI Registry instead of direct SDK-to-Registry pushes, adjust example
commands/snippets and any configuration guidance to reference the sidecar
endpoint (EvalHub) and sidecar flow, and ensure references to symbols like
EvalHub, Registry, S3/S4/A4/A5 align with the diagram language used elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
185-198:⚠️ Potential issue | 🟠 MajorREADME still has stale callback examples that conflict with this new canonical path.
This section correctly promotes
DefaultCallbacks.from_adapter(adapter), but later snippets still show directDefaultCallbacks(...)usage with unsupported params (registry_url,registry_username,registry_password) andjob_spec.job_idinstead ofjob_spec.id. That makes the README internally inconsistent and can lead users to broken integration code.Suggested doc alignment (replace stale direct-construction snippets)
-# Create callbacks -callbacks = DefaultCallbacks( - job_id=job_spec.job_id, - benchmark_id=job_spec.benchmark_id, - benchmark_index=job_spec.benchmark_index, - sidecar_url=job_spec.callback_url, - registry_url=settings.registry_url, - registry_username=settings.registry_username, - registry_password=settings.registry_password, - insecure=settings.registry_insecure, -) +# Create callbacks from adapter (recommended) +callbacks = DefaultCallbacks.from_adapter(adapter)-# Run adapter -results = adapter.run_benchmark_job(job_spec, callbacks) +# Run adapter +results = adapter.run_benchmark_job(adapter.job_spec, callbacks)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 185 - 198, The README contains stale examples that instantiate DefaultCallbacks directly with unsupported parameters and use job_spec.job_id; update the examples to use DefaultCallbacks.from_adapter(adapter) (referencing DefaultCallbacks.from_adapter and adapter) instead of direct DefaultCallbacks(...) and remove any unsupported args like registry_url, registry_username, registry_password; also replace job_spec.job_id occurrences with job_spec.id so the snippets match the current API and the new canonical usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@README.md`:
- Around line 185-198: The README contains stale examples that instantiate
DefaultCallbacks directly with unsupported parameters and use job_spec.job_id;
update the examples to use DefaultCallbacks.from_adapter(adapter) (referencing
DefaultCallbacks.from_adapter and adapter) instead of direct
DefaultCallbacks(...) and remove any unsupported args like registry_url,
registry_username, registry_password; also replace job_spec.job_id occurrences
with job_spec.id so the snippets match the current API and the new canonical
usage.
julpayne
approved these changes
Mar 27, 2026
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
now OCI Artifact Push goes through sidecar for Authentication
What and why
Closes #
Type
Testing
Breaking changes
Summary by CodeRabbit