Update to LMEval allowOnline setting instructions #901
Conversation
|
Warning Rate limit exceeded@BSynRedhat has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis update introduces new documentation and procedures for enabling external resource access and remote code execution for LMEval Jobs. It adds detailed CLI and console instructions, centralizes this guidance in a new assembly, and streamlines the original operator-level settings documentation by removing procedural details now covered elsewhere. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI or Console
participant ConfigMap
participant TrustyAI Operator
participant LMEvalJob
User->>CLI or Console: Initiate configuration update
CLI or Console->>ConfigMap: Patch ConfigMap (set allowOnline/allowCodeExecution)
CLI or Console->>TrustyAI Operator: Restart deployment
TrustyAI Operator->>ConfigMap: Read updated settings
User->>LMEvalJob: Submit job with allowOnline/allowCodeExecution fields
LMEvalJob->>TrustyAI Operator: Request external resource access
TrustyAI Operator->>LMEvalJob: Permit access based on config
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
modules/setting-up-lmeval.adoc (1)
75-83: Ensure ConfigMap key islmes-allow-code-execution, notlmes-code-executionThe console and CLI procedures use
lmes-allow-code-execution, so we should update all docs for consistency:• modules/setting-up-lmeval.adoc (around line 79): change
lmes-code-execution→lmes-allow-code-execution
• modules/using-llm-as-a-judge-metrics-with-lmeval.adoc (around line 68): changelmes-code-execution→lmes-allow-code-executionReplace the table rows in each file as follows:
-| `lmes-code-execution` -| true -| Determines whether LMEval jobs can set the `trust remote code` mode to `on`. +| `lmes-allow-code-execution` +| true +| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.modules/updating-lmeval-job-configuration-using-the-cli.adoc (1)
77-83: Missing step: set per-joballowOnline/allowCodeExecutionon the LMEvalJobCluster-level switches alone are not sufficient. Users must also set the flags on their LMEvalJob resource.
oc rollout restart deployment trustyai-service-operator-controller-manager -n redhat-ods-applications ---- + +. Enable online access and/or code execution on the specific `LMEvalJob`: ++ +[source,sh] +---- +# Patch an existing job (example name: example-lmeval) +oc patch lmevaljob example-lmeval -n <user-or-target-namespace> \ + --type merge -p '{"spec":{"allowOnline":true,"allowCodeExecution":true}}' +---- ++ +[source,yaml] +---- +# Or define it in YAML when creating the job +apiVersion: trustyai.opendatahub.io/v1alpha1 +kind: LMEvalJob +metadata: + name: example-lmeval + namespace: <user-or-target-namespace> +spec: + allowOnline: true + allowCodeExecution: true +----
🧹 Nitpick comments (4)
assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc (1)
8-11: Tighten wording, link style, and clarify independent togglesMinor grammar and clarity improvements; also make it clear that online access and code execution can be enabled independently.
- LMEval Jobs do not allow internet access or remote code execution by default. When configuring an `LMEvalJob`, it may require access to external resources, for example task datasets and model tokenizers, usually hosted on link:https://huggingface.co[Hugging Face]. If you trust the source and have reviewed the content of these artifacts, `LMEvalJob` can be configured to automatically download them. + LMEval jobs do not allow internet access or remote code execution by default. When configuring an `LMEvalJob`, it might require access to external resources, for example, task datasets and model tokenizers, usually hosted on link:https://huggingface.co[Hugging Face^]. If you trust the source and have reviewed the content of these artifacts, LMEval jobs can be configured to automatically download them. -Follow the steps below to enable online access and remote code execution for LMEval Jobs. Choose to update these settings by using either the CLI or in the console. +Follow the steps below to enable online access and/or remote code execution for LMEval jobs. Choose to update these settings by using either the CLI or the console. Enable only what you require for your use case.modules/updating-lmeval-job-configuration-using-the-console.adoc (1)
53-66: Tighten step wording; remove duplicationThe sentence duplicates “in the LMEvalJob custom resource”.
-. Add the following fields to the `LMEvalJob` custom resource to enable online access in the LMEvalJob custom resource: +. Add the following fields to the `LMEvalJob` to enable online access and code execution for this job:modules/updating-lmeval-job-configuration-using-the-cli.adoc (2)
9-9: Minor wording“In the CLI” → “using the CLI”.
-Follow these steps to enable online access (`allowOnline`) and remote code execution (`allowCodeExecution`) modes in the CLI for LMEval Jobs. +Follow these steps to enable online access (`allowOnline`) and remote code execution (`allowCodeExecution`) modes using the CLI for LMEval Jobs.
57-59: Clarify you can enable either capability independentlyReaders may think both must be set to true together. Suggest clarifying “and/or” and showing how to set only one.
-. Update the settings for the TrustyAI operator to enable external connectivity and remote code execution: +. Update the settings for the TrustyAI operator to enable external connectivity and/or remote code execution: @@ -oc patch configmap trustyai-service-operator-config -n redhat-ods-applications \ ---type merge -p '{"data":{"lmes-allow-online":"true","lmes-allow-code-execution":"true"}}' +oc patch configmap trustyai-service-operator-config -n redhat-ods-applications \ +--type merge -p '{"data":{"lmes-allow-online":"true","lmes-allow-code-execution":"true"}}' +# Example: enable online only +# oc patch configmap trustyai-service-operator-config -n redhat-ods-applications \ +# --type merge -p '{"data":{"lmes-allow-online":"true"}}'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (5)
assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc(1 hunks)assemblies/evaluating-large-language-models.adoc(1 hunks)modules/setting-up-lmeval.adoc(1 hunks)modules/updating-lmeval-job-configuration-using-the-cli.adoc(1 hunks)modules/updating-lmeval-job-configuration-using-the-console.adoc(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: bredamc
PR: opendatahub-io/opendatahub-documentation#860
File: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:62-65
Timestamp: 2025-07-16T09:05:50.422Z
Learning: In the opendatahub-documentation project, bredamc prefers comprehensive error detection in code reviews. Continue to point out potential errors whether they are related to the code changes in the PR or not, but clearly distinguish between errors introduced in the PR versus errors in existing code.
🔇 Additional comments (3)
assemblies/evaluating-large-language-models.adoc (2)
20-21: Placement of new section looks correctIncluding the new section between setup and job execution reads well and keeps the flow logical.
20-21: Confirm assembly-in-assembly include is supported by your build pipelineYou’re including an ASSEMBLY file inside another ASSEMBLY. Some pipelines expect assemblies to include modules only. Please confirm this builds cleanly; otherwise, consider inlining the two procedure modules here or converting the included file to a CONCEPT/REFERENCE module.
modules/updating-lmeval-job-configuration-using-the-cli.adoc (1)
57-83: Add a post-patch verification snippet to confirm settingsTo ensure your ConfigMap update and rollout took effect, append the following under the restart step in
modules/updating-lmeval-job-configuration-using-the-cli.adoc:[source,sh]
Verify ConfigMap values in your cluster
oc get cm trustyai-service-operator-config -n redhat-ods-applications
-o jsonpath='{.data.lmes-allow-online}{"\n"}{.data.lmes-allow-code-execution}{"\n"}'Verify a sample LMEvalJob has the flags set
oc get lmevaljob -n
-o jsonpath='{.spec.allowOnline}{"\n"}{.spec.allowCodeExecution}{"\n"}'Replace
<example-lmeval-name>and<namespace>with your actual job name and namespace, then run these commands against your cluster to confirm bothlmes-allow-onlineandlmes-allow-code-executionare enabled.
| The `allowOnline` and `allowCodeExecution` settings are *disabled* by default at the operator level in {productname-long}. LMEval jobs do not have internet access or permission to run any externally downloaded code unless explicitly enabled. | ||
| -- | ||
|
|
||
| .Enabling allowOnline and allowCodeExecution modes | ||
|
|
||
| You can set your `LMEvalJob` to access online content by updating and patching the TrustyAI operator `ConfigMap`. | ||
|
|
||
| . In the `ConfigMap`, change the `opendatahub.io/managed` setting to `'false'`. | ||
|
|
||
| . Patch the TrustyAI operator `ConfigMap` with the following `kubectl` code: | ||
| + | ||
| [source] | ||
| ---- | ||
| kubectl patch configmap trustyai-service-operator-config -n redhat-ods-applications \ | ||
| --type merge -p '{"data":{"lmes-allow-online":"true","lmes-allow-code-execution":"true"}}' | ||
| ---- | ||
| + | ||
| . Restart the TrustyAI operator with the following `kubectl` command: | ||
| + | ||
| [source] | ||
| ---- | ||
| kubectl rollout restart deployment trustyai-service-operator-controller-manager -n redhat-ods-applications | ||
| ---- | ||
|
|
||
| endif::[] | ||
|
|
||
| endif::[] |
There was a problem hiding this comment.
Default-state note conflicts with the table above and key naming is inconsistent
The downstream IMPORTANT note says both settings are disabled by default, but the table above lists:
lmes-allow-onlinedefault astruelmes-code-executiondefault astrue
Additionally, the new procedures refer to the ConfigMap key lmes-allow-code-execution, while the table lists lmes-code-execution. Please align defaults and key names.
Recommended:
- Use
lmes-allow-code-execution(to match the new procedures). - For downstream (
ifndef::upstream[]), set both defaults tofalseor conditionally render defaults per upstream/downstream.
Here are concrete edits for the table rows (also resolves the key-name mismatch):
-| `lmes-allow-online`
-| true
+| `lmes-allow-online`
+| false
| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet.
-
-| `lmes-code-execution`
-| true
-| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
+
+| `lmes-allow-code-execution`
+| false
+| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.If upstream must remain enabled by default, gate the defaults:
-| `lmes-allow-online`
-| false
+ifdef::upstream[]
+| `lmes-allow-online`
+| true
+endif::[]
+ifndef::upstream[]
+| `lmes-allow-online`
+| false
+endif::[]
| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet.
-
-| `lmes-code-execution`
-| false
-| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
+ifdef::upstream[]
+| `lmes-allow-code-execution`
+| true
+endif::[]
+ifndef::upstream[]
+| `lmes-allow-code-execution`
+| false
+endif::[]
+| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In modules/setting-up-lmeval.adoc around lines 102 to 104, the default values
for `allowOnline` and `allowCodeExecution` settings conflict between the note
and the table, and key names are inconsistent. Update the table to use
`lmes-allow-code-execution` instead of `lmes-code-execution` to match the new
procedures, and set both defaults to false for downstream or conditionally
render defaults based on upstream/downstream context to align with the note
stating they are disabled by default.
There was a problem hiding this comment.
I'm not quite sure what it's saying here
There was a problem hiding this comment.
lmes-allow-code-execution is the key that we need to be using, so we should make sure it's consistent
There was a problem hiding this comment.
@RobGeada
Short version: the table says both settings default to true, but the downstream IMPORTANT note says they’re disabled by default. Also ensure the key name is lmes-allow-code-execution everywhere.
Replace the two rows in the “LM-Eval properties” table with the conditional defaults below to align with the notes:
ifdef::upstream[]
| `lmes-allow-online`
| true
| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet.
endif::[]
ifndef::upstream[]
| `lmes-allow-online`
| false
| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet.
endif::[]
ifdef::upstream[]
| `lmes-allow-code-execution`
| true
| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
endif::[]
ifndef::upstream[]
| `lmes-allow-code-execution`
| false
| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
endif::[]If you prefer a simpler approach for downstream-only docs, set both defaults to false unconditionally. Want me to also update the upstream IMPORTANT note to mention allowCodeExecution for completeness?
🧠 Learnings used
Learnt from: bredamc
PR: opendatahub-io/opendatahub-documentation#860
File: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:62-65
Timestamp: 2025-07-16T09:05:50.422Z
Learning: In the opendatahub-documentation project, bredamc prefers comprehensive error detection in code reviews. Continue to point out potential errors whether they are related to the code changes in the PR or not, but clearly distinguish between errors introduced in the PR versus errors in existing code.
There was a problem hiding this comment.
🧩 Analysis chain
@coderabbit-user Thanks, agreed. I confirmed we should standardize on lmes-allow-code-execution everywhere.
- Repo check: no uses of lmes-code-execution were found; modules/setting-up-lmeval.adoc already uses lmes-allow-code-execution.
- One remaining issue is default values: the table lists both as true unconditionally, while the downstream IMPORTANT note says they’re disabled by default. To align, here’s a targeted edit that keeps the key name consistent and gates defaults per upstream/downstream:
-| `lmes-allow-online`
-| true
+ifdef::upstream[]
+| `lmes-allow-online`
+| true
+endif::[]
+ifndef::upstream[]
+| `lmes-allow-online`
+| false
+endif::[]
| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet.
-
-| `lmes-allow-code-execution`
-| true
+ifdef::upstream[]
+| `lmes-allow-code-execution`
+| true
+endif::[]
+ifndef::upstream[]
+| `lmes-allow-code-execution`
+| false
+endif::[]
| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.If you prefer a simpler downstream-only fix, we can instead set both defaults to false (no gating). Happy to update accordingly.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for deprecated key name 'lmes-code-execution'..."
rg -n --no-heading $'lmes-code-execution' || true
echo
echo "Searching for correct key name 'lmes-allow-code-execution'..."
rg -n --no-heading $'lmes-allow-code-execution' || true
echo
echo "Show the table section in modules/setting-up-lmeval.adoc around allow-online/code-execution keys for context:"
rg -n -A4 -B4 $'lmes-allow-online|lmes-code-execution|lmes-allow-code-execution' modules/setting-up-lmeval.adoc || true
Length of output: 1660
@coderabbit-user Confirmed. There’s one remaining use of the deprecated key and the defaults in the table still read true.
Findings from the repo scan:
- Deprecated key found: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc Line 68
- Table shows defaults as true: modules/setting-up-lmeval.adoc Lines 76 and 80
Proposed fixes:
- Replace the deprecated key (Line 68)
- "lmes-code-execution": "true",
+ "lmes-allow-code-execution": "true",- Align table defaults with upstream/downstream notes and keep the correct key name
- | `lmes-allow-online`
- | true
- | Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet.
-
- | `lmes-allow-code-execution`
- | true
- | Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
+ifdef::upstream[]
+| `lmes-allow-online`
+| true
+| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet.
+
+| `lmes-allow-code-execution`
+| true
+| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
+endif::[]
+
+ifndef::upstream[]
+| `lmes-allow-online`
+| false
+| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet.
+
+| `lmes-allow-code-execution`
+| false
+| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
+endif::[]If upstream intends allow-code-execution to be disabled by default, I can flip that upstream default to false—let me know your preference.
✏️ Learnings added
Learnt from: RobGeada
PR: opendatahub-io/opendatahub-documentation#901
File: modules/setting-up-lmeval.adoc:102-104
Timestamp: 2025-08-08T15:08:58.763Z
Learning: In opendatahub-io/opendatahub-documentation, the correct ConfigMap key is `lmes-allow-code-execution` (not `lmes-code-execution`); ensure consistency across all modules and examples.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: bredamc
PR: opendatahub-io/opendatahub-documentation#860
File: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:62-65
Timestamp: 2025-07-16T09:05:50.422Z
Learning: In the opendatahub-documentation project, bredamc prefers comprehensive error detection in code reviews. Continue to point out potential errors whether they are related to the code changes in the PR or not, but clearly distinguish between errors introduced in the PR versus errors in existing code.
modules/updating-lmeval-job-configuration-using-the-console.adoc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
modules/updating-lmeval-job-configuration-using-the-cli.adoc (4)
11-14: Enhance the security advisory with concrete risks and guidanceConsider briefly calling out typical risks (untrusted code execution, data exfiltration via outbound egress) and recommending mitigations (restrict egress via NetworkPolicy/EgressFirewall, use allowlists).
Apply this minimal tweak:
[IMPORTANT] ==== -Enabling online access and code execution involves a security risk. Only use these configurations if you trust the source(s). +Enabling online access and code execution introduces security risk (for example, untrusted code execution, outbound data exfiltration). +Only enable these settings when strictly required, and prefer allowlisted sources with restricted egress (for example, NetworkPolicy/EgressFirewall). ====
39-53: Clarify the “Effect” of theopendatahub.io/managedannotation“Allows the user to manage the TrustyAI operator” is vague. The practical effect is about reconcilers not overwriting manual edits to the resource.
| `opendatahub.io/managed` | `true` -| User changes are not allowed and will be automatically reverted. +| The ODH reconciler manages this resource; manual changes may be reverted automatically. @@ | `opendatahub.io/managed` | `false` -| Allows the user to manage the TrustyAI operator. +| Disables ODH reconciliation for this resource, allowing manual changes to persist.
62-76: Clarify that ConfigMap keys act as cluster-level gates; per-job flags are still requiredAvoid the impression that setting the ConfigMap alone enables the features by default for all jobs.
| `lmes-allow-online` | `false` -| Enables internet access for LMEval jobs at the cluster level. +| Permits (gates) internet access for LMEval jobs cluster-wide. +| Effective only when `spec.allowOnline` is also set on the individual `LMEvalJob`. @@ | `lmes-allow-code-execution` | `false` -| Allows execution of code downloaded alongside datasets or tokenizers. +| Permits (gates) remote code execution for artifacts downloaded by LM-Eval (for example, datasets, tokenizers). +| Effective only when `spec.allowCodeExecution` is also set on the `LMEvalJob`.
30-31: Avoid hard-coding the operator namespace in multiple commandsConsider defining a namespace attribute once (for example,
:odh-app-namespace: redhat-ods-applications) and using-n {odh-app-namespace}across commands to reduce duplication and ease maintenance.Also applies to: 58-59, 81-82
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc(1 hunks)modules/setting-up-lmeval.adoc(2 hunks)modules/updating-lmeval-job-configuration-using-the-cli.adoc(1 hunks)modules/updating-lmeval-job-configuration-using-the-console.adoc(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc
- modules/setting-up-lmeval.adoc
- modules/updating-lmeval-job-configuration-using-the-console.adoc
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: bredamc
PR: opendatahub-io/opendatahub-documentation#860
File: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:62-65
Timestamp: 2025-07-16T09:05:50.422Z
Learning: In the opendatahub-documentation project, bredamc prefers comprehensive error detection in code reviews. Continue to point out potential errors whether they are related to the code changes in the PR or not, but clearly distinguish between errors introduced in the PR versus errors in existing code.
🔇 Additional comments (3)
modules/updating-lmeval-job-configuration-using-the-cli.adoc (3)
26-37: ✅ Verified correct ConfigMap for LMEval settingsA repo-wide search confirms that:
- The
trustyai-service-operator-configConfigMap is explicitly documented as the source for LM-Eval global settings (see modules/setting-up-lmeval.adoc:38).- Several LMEval modules reference and patch this ConfigMap (e.g. updating-lmeval-job-configuration-using-the-cli.adoc).
- The
opendatahub.io/managedannotation is used throughout to toggle operator management.No changes required.
77-82: Ignore incorrect operator suggestion—TrustyAI operator manages LMEvalJob
LMEvalJob CRDs are reconciled by the TrustyAI Kubernetes operator (see modules/lmeval-evaluation-job.adoc), so restarting thetrustyai-service-operator-controller-managerdeployment is the correct step. No changes required.Likely an incorrect or invalid review comment.
54-61: Double-check LMEval ConfigMap settings
- The keys
lmes-allow-onlineandlmes-allow-code-executionare used consistently across the documentation and match exactly.- These settings are only referenced in docs (e.g. modules/setting-up-lmeval.adoc, modules/updating-lmeval-job-configuration-using-the-cli.adoc).
- There are no code references in this repo to validate the owning component; please manually confirm that the TrustyAI operator’s cluster-level ConfigMap named
trustyai-service-operator-configactually defines these keys for controlling LMEval jobs.
Update to insert a section between current 6.1 and 6.2 about allowing offline access to online and external code sources.
Summary by CodeRabbit