Building orig-edivisive from scratch#79283
Conversation
WalkthroughThe Orion CI step was changed to run an experimental Python-based implementation checked out from the ChangesOrion experimental run: fetch, install, execute, and artifact
Sequence DiagramsequenceDiagram
participant CI as CI Script
participant Git as Git Remote
participant Python as Python Env
participant Art as Artifacts Dir
CI->>Git: fetch refs/tags/orig-edivisive-exp
Git-->>CI: tag commit
CI->>CI: git checkout fetched commit
CI->>Python: pip install -r requirements.txt && pip install .
Python-->>CI: environment ready
CI->>Python: run `orion --viz ...`
Python-->>Art: write viz to `$EXP_DIR/orion-exp-output.txt`
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vishnuchalla The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/pj-rehearse periodic-ci-quay-quay-tests-master-quay-performance-quay-e2e-tests-quay317-performance-test |
|
@vishnuchalla: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-netobserv-netobserv-perf-tests-main-netobserv-aws-4.22-nightly-x86-node-density-heavy-25nodes |
|
@vishnuchalla: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh`:
- Line 311: The orion invocation uses unquoted scalars and a plain EXTRA_FLAGS
string which can break on spaces/special chars; change the script to quote
scalar expansions (use "${ORION_CONFIG}", "${IGNORE_JOB_ITERATIONS}", and quote
the tee target "${EXP_DIR}/orion-exp-output.txt") and treat EXTRA_FLAGS as an
array (declare/construct EXTRA_FLAGS as an array earlier and expand it here as
"${EXTRA_FLAGS[@]}") so the command becomes robust to spaces and special
characters while preserving the pipe and the || true behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5802fcb7-67d1-4be0-94bd-40015991d65a
📒 Files selected for processing (1)
ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh
|
|
||
| echo "Running experimental orion (original e-divisive)..." | ||
| "$EXP_BINARY" --node-count ${IGNORE_JOB_ITERATIONS} --config ${ORION_CONFIG} ${EXTRA_FLAGS} --viz | tee "$EXP_DIR/${FILENAME}.txt" || true | ||
| orion --node-count ${IGNORE_JOB_ITERATIONS} --config ${ORION_CONFIG} ${EXTRA_FLAGS} --viz | tee "$EXP_DIR/orion-exp-output.txt" || true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and get the total line count
wc -l ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.shRepository: openshift/release
Length of output: 141
🏁 Script executed:
# View the context around line 311
sed -n '300,320p' ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh | cat -nRepository: openshift/release
Length of output: 1107
🏁 Script executed:
# Check the broader experimental section and variable definitions
sed -n '290,330p' ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh | cat -nRepository: openshift/release
Length of output: 1705
🏁 Script executed:
# Search for where these variables are set to understand their source
rg "IGNORE_JOB_ITERATIONS|ORION_CONFIG|EXTRA_FLAGS" ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh -B 2 -A 1Repository: openshift/release
Length of output: 2242
Quote variables and use array expansion for flags in the experimental orion invocation (Line 311).
The unquoted variable expansions can break when ORION_CONFIG contains spaces or special characters (e.g., downloaded file paths). Quote scalar values and convert EXTRA_FLAGS to an array for safe expansion.
Suggested fix
- orion --node-count ${IGNORE_JOB_ITERATIONS} --config ${ORION_CONFIG} ${EXTRA_FLAGS} --viz | tee "$EXP_DIR/orion-exp-output.txt" || true
+ extra_args=()
+ if [[ -n "${EXTRA_FLAGS}" ]]; then
+ # shellcheck disable=SC2206 # intentional split of prebuilt flags string
+ extra_args=( ${EXTRA_FLAGS} )
+ fi
+ orion --node-count "${IGNORE_JOB_ITERATIONS}" --config "${ORION_CONFIG}" "${extra_args[@]}" --viz | tee "$EXP_DIR/orion-exp-output.txt" || true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| orion --node-count ${IGNORE_JOB_ITERATIONS} --config ${ORION_CONFIG} ${EXTRA_FLAGS} --viz | tee "$EXP_DIR/orion-exp-output.txt" || true | |
| extra_args=() | |
| if [[ -n "${EXTRA_FLAGS}" ]]; then | |
| # shellcheck disable=SC2206 # intentional split of prebuilt flags string | |
| extra_args=( ${EXTRA_FLAGS} ) | |
| fi | |
| orion --node-count "${IGNORE_JOB_ITERATIONS}" --config "${ORION_CONFIG}" "${extra_args[@]}" --viz | tee "$EXP_DIR/orion-exp-output.txt" || true |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 311-311: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 311-311: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 311-311: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh`
at line 311, The orion invocation uses unquoted scalars and a plain EXTRA_FLAGS
string which can break on spaces/special chars; change the script to quote
scalar expansions (use "${ORION_CONFIG}", "${IGNORE_JOB_ITERATIONS}", and quote
the tee target "${EXP_DIR}/orion-exp-output.txt") and treat EXTRA_FLAGS as an
array (declare/construct EXTRA_FLAGS as an array earlier and expand it here as
"${EXTRA_FLAGS[@]}") so the command becomes robust to spaces and special
characters while preserving the pipe and the || true behavior.
Signed-off-by: Vishnu Challa <vchalla@redhat.com>
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
vishnuchalla
changed the title
|
/pj-rehearse periodic-ci-netobserv-netobserv-perf-tests-main-netobserv-aws-4.22-nightly-x86-node-density-heavy-25nodes |
|
@vishnuchalla: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@vishnuchalla: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/assign @chentex |
|
/pj-rehearse ack |
|
@vishnuchalla: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
3 participants
Description
Updating orig-edivise source code for orion
Testing
Will be verified through rehearsal
Updates the OpenShift CI step for OpenShift QE's Orion workflow (ci-operator/step-registry/openshift-qe/orion) to run the experimental "original e-divisive" Orion implementation from source instead of executing a downloaded prebuilt orion-amd64 binary. The step now clones the orion repo at the orig-edivisive-exp tag, installs Python requirements, and runs the Python-based Orion, writing visualization output for the experimental run to a fixed experimental artifact file ($EXP_DIR/orion-exp-output.txt) to keep experimental results isolated from the standard Orion artifacts. No public API changes. The author has triggered rehearsal runs to validate the change.