Skip to content

fix: add no-op for --collect-must-gather in older versions#541

Closed
adolfo-ab wants to merge 11 commits intoopendatahub-io:mainfrom
adolfo-ab:collect-mock-gather
Closed

fix: add no-op for --collect-must-gather in older versions#541
adolfo-ab wants to merge 11 commits intoopendatahub-io:mainfrom
adolfo-ab:collect-mock-gather

Conversation

@adolfo-ab
Copy link
Copy Markdown
Contributor

@adolfo-ab adolfo-ab commented Aug 21, 2025

add no-op for --collect-must-gather in older versions

Description

We want devops to use --collect-must-gather by default, but backporting it isn't trivial. For now we want to add a mock option that does nothing to avoid breaking runs with older branches.

How Has This Been Tested?

Running locally with/without --collect-must-gather in different branches.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Summary by CodeRabbit

  • New Features

    • PRs now auto-assign the author and receive a welcome comment.
    • On PR merge, container images are automatically built and pushed with branch-based tags, with status posted to the PR.
    • Pytest now supports --upgrade-deployment-modes to run selected deployment-mode upgrade tests.
  • Tests

    • Added pre/post upgrade tests for Model Registry; streamlined client utilities.
    • Applied per-test marks for model serving upgrade scenarios.
  • Documentation

    • Updated contributor docs and upgrade test README with new automation and CLI flag.
  • Chores

    • Updated CI token usage for labeling workflow.
    • Added pytest-html for HTML test reports.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Aug 21, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Updates GitHub workflows (token use, welcome-assignee behavior, new build/push-on-merge pipeline), adjusts README/docs, and extends workflow scripts to assign PR authors. Test infrastructure gains upgrade-mode gating, new pytest options, and Model Registry test refactor with new utilities and upgrade tests. Minor fixtures and security context tweaks included.

Changes

Cohort / File(s) Summary
Docs
.github/README.md, tests/model_serving/model_server/upgrade/README.md
Updated instructions: assign PR to author; added docs for --upgrade-deployment-modes usage.
GitHub Actions — Tokens & Naming
.github/workflows/add-remove-labels.yml, .github/workflows/add-welcome-comment-set-assignee.yml
Switched labeling workflow to use secrets.RHODS_CI_BOT_PAT; renamed welcome job and aligned ACTION env to include assignee setting.
GitHub Actions — New Build on Merge
.github/workflows/build-push-container-on-merge.yml
Added workflow to build and push container to Quay on PR merge, with PR comment summarizing build/push outcomes.
Workflow Scripts
.github/workflows/scripts/constants.py, .github/workflows/scripts/pr_workflow.py
Welcome message mentions assigning author; implementation now comments and assigns PR author with error handling.
Pytest Configuration
conftest.py, pyproject.toml
Added --upgrade-deployment-modes and --collect-must-gather options; upgrade test selection gating; added pytest-html dependency.
Model Explainability Fixtures
tests/model_explainability/lm_eval/conftest.py, tests/model_explainability/trustyai_service/conftest.py
Adjusted task template structure to dict; added hardened securityContext for MinIO container.
Model Registry — Constants & Utils
tests/model_registry/constants.py, tests/model_registry/utils.py
Added MR constants/endpoints; expanded utilities (pod status helpers, wait logic, client helpers); API tweaks for service/endpoint helpers; added ModelRegistryV1Alpha1.
Model Registry — Tests Refactor
tests/model_registry/conftest.py, tests/model_registry/test_model_registry_creation.py, tests/model_registry/upgrade/test_model_registry_upgrade.py, tests/model_registry/test_rest_api.py
Comprehensive fixture overhaul for upgrade-aware flows, REST client usage, and DSC patching; added pre/post upgrade tests; removed schemathesis REST API test.
Model Serving Upgrade Tests
tests/model_serving/model_server/upgrade/test_upgrade.py
Replaced module-level marks with per-test marks for serverless/rawdeployment/modelmesh.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor GitHub
  participant Workflow as Build/Push On PR Merge
  participant Quay as Quay Registry
  participant PR as Pull Request

  GitHub->>Workflow: pull_request_target (closed)
  alt PR is merged
    Workflow->>Workflow: Determine TAG (main -> latest else base ref)
    Workflow->>Workflow: Build image (Buildah)
    Workflow->>Quay: Push image (image:tag)
    Workflow->>PR: Comment build/push status
  else Not merged
    Note over Workflow: Job skipped (if condition)
  end
Loading
sequenceDiagram
  autonumber
  actor GH as GitHub
  participant Job as add-welcome-comment-set-assignee
  participant API as GitHub API

  GH->>Job: PR opened
  Job->>API: Post welcome comment
  Job->>API: Assign PR author as assignee
  alt UnknownObjectException
    Job->>Job: Log warning and continue
  end
Loading
sequenceDiagram
  autonumber
  participant Pytest as PyTest Collector
  participant CLI as CLI Options
  participant Tests as Collected Items

  CLI-->>Pytest: --pre/--post and --upgrade-deployment-modes
  Pytest->>Tests: Inspect keywords (pre_upgrade/post_upgrade + modes)
  alt Modes provided
    Pytest->>Tests: Include pre/post only if keyword matches mode list
  else No modes
    Pytest->>Tests: Include all pre/post as usual
  end
  Pytest->>Pytest: Deselect non-matching upgrade tests
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

A rabbit taps the merge with cheer,
New tags hop out when PRs appear. 🐇
A welcome note, “You’re set, assigned!”
Tests now sift by upgrade kind.
Registries march in tidy rows,
Where utils sniff pod status woes—
Ship to Quay, and off it goes! 🚀

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 actionlint (1.7.7)
.github/workflows/add-remove-labels.yml

could not read ".github/workflows/add-remove-labels.yml": open .github/workflows/add-remove-labels.yml: no such file or directory

.github/workflows/add-welcome-comment-set-assignee.yml

could not read ".github/workflows/add-welcome-comment-set-assignee.yml": open .github/workflows/add-welcome-comment-set-assignee.yml: no such file or directory

.github/workflows/build-push-container-on-merge.yml

could not read ".github/workflows/build-push-container-on-merge.yml": open .github/workflows/build-push-container-on-merge.yml: no such file or directory

🔧 YAMLlint (1.37.1)
.github/workflows/add-remove-labels.yml

[Errno 2] No such file or directory: '.github/workflows/add-remove-labels.yml'

.github/workflows/add-welcome-comment-set-assignee.yml

[Errno 2] No such file or directory: '.github/workflows/add-welcome-comment-set-assignee.yml'

.github/workflows/build-push-container-on-merge.yml

[Errno 2] No such file or directory: '.github/workflows/build-push-container-on-merge.yml'

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d7e08d9 and bc52885.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • .github/README.md (1 hunks)
  • .github/workflows/add-remove-labels.yml (1 hunks)
  • .github/workflows/add-welcome-comment-set-assignee.yml (3 hunks)
  • .github/workflows/build-push-container-on-merge.yml (1 hunks)
  • .github/workflows/scripts/constants.py (1 hunks)
  • .github/workflows/scripts/pr_workflow.py (2 hunks)
  • conftest.py (5 hunks)
  • pyproject.toml (1 hunks)
  • tests/model_explainability/lm_eval/conftest.py (1 hunks)
  • tests/model_explainability/trustyai_service/conftest.py (1 hunks)
  • tests/model_registry/conftest.py (4 hunks)
  • tests/model_registry/constants.py (1 hunks)
  • tests/model_registry/test_model_registry_creation.py (2 hunks)
  • tests/model_registry/test_rest_api.py (0 hunks)
  • tests/model_registry/upgrade/test_model_registry_upgrade.py (1 hunks)
  • tests/model_registry/utils.py (3 hunks)
  • tests/model_serving/model_server/upgrade/README.md (1 hunks)
  • tests/model_serving/model_server/upgrade/test_upgrade.py (8 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee
  • Build image based on the PR

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
    and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
  • To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
    pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.
Supported labels

{'/wip', '/lgtm', '/verified', '/cherry-pick', '/build-push-pr-image', '/hold'}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants