Skip to content

Conversation

@are-ces
Copy link
Contributor

@are-ces are-ces commented Oct 31, 2025

Description

RHAIIS Model is defined in GH env variables instead of in code. This way we don't need to create a PR each time the model changes.

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Chores
    • Updated CI/CD pipeline configuration to use environment variables for model selection and streamlined environment variable management across test steps for improved flexibility and maintainability.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Walkthrough

The RHAIIS e2e test workflow is refactored to use a configurable model variable instead of a hardcoded value, and per-step environment variable declarations are removed in favor of job-level definitions.

Changes

Cohort / File(s) Summary
Workflow configuration updates
.github/workflows/e2e_tests_rhaiis.yaml
Replaces hardcoded RHAIIS_MODEL value with variable reference ${{ vars.RHAIIS_MODEL }}. Removes per-step RHAIIS_URL and RHAIIS_API_KEY environment declarations from "Test RHAIIS connectivity" and "Run service manually" steps, relying on job-level environment variable definitions instead.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file with straightforward configuration changes
  • Verify that job-level environment variables are sufficient for all affected steps
  • Confirm the variable reference syntax is correct and the variable exists in repository settings

Possibly related PRs

Suggested reviewers

  • tisnik
  • radofuchs

Poem

🐰 Variables flow through the workflow stream,
No more hardcoded dreams,
Job-level clarity takes the stage,
Configuration turns a cleaner page! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix: RHAIIS model parameter" directly relates to the main change in the changeset. According to the PR objectives, the primary goal is to move the RHAIIS model from a hardcoded value in the workflow to a GitHub environment variable, allowing the model to be changed without requiring a PR. The title accurately captures this parametrization of the RHAIIS model. While there are secondary changes to remove per-step environment definitions, the title appropriately focuses on the primary objective. The title is concise, clear, and uses conventional commit format, making it easy for a teammate to understand the main change when scanning pull request history.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 980e871 and 90cc40a.

📒 Files selected for processing (1)
  • .github/workflows/e2e_tests_rhaiis.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: build-pr
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: e2e_tests (azure)
  • GitHub Check: e2e_tests (ci)
🔇 Additional comments (2)
.github/workflows/e2e_tests_rhaiis.yaml (2)

121-137: No issues found—job-level environment variables are properly inherited by steps.

The job-level environment variables RHAIIS_URL and RHAIIS_API_KEY are defined at lines 18–19. In GitHub Actions, all steps inherit job-level environment variables automatically, so the removed step-level env blocks were redundant and do not break the connectivity steps. The curl command and service startup steps will correctly resolve ${RHAIIS_URL} and ${RHAIIS_API_KEY} from the job-level definitions.


20-20: GitHub organization/repository variable RHAIIS_MODEL must be configured before this PR merges.

Line 20 references ${{ vars.RHAIIS_MODEL }}. If this variable is not configured in GitHub repository or organization settings, it will evaluate to an empty string at runtime, causing the test to fail when the test configuration attempts to use an undefined model ID.

The variable IS actively used: it flows through the workflow job environment → docker-compose.yaml (line 20 injects it as RHAIIS_MODEL=${RHAIIS_MODEL}) → container environment → tests/e2e/configs/run-rhaiis.yaml (lines 141, 144 consume it as ${env.RHAIIS_MODEL}). Confirm the variable has been created in repository settings before merging.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@radofuchs radofuchs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@radofuchs radofuchs merged commit ea85a90 into lightspeed-core:main Oct 31, 2025
20 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants