Skip to content

updated_catalog_config_map should wait for the catalog api to become available#987

Merged
dbasunag merged 3 commits intoopendatahub-io:mainfrom
dbasunag:minor_bug
Jan 7, 2026
Merged

updated_catalog_config_map should wait for the catalog api to become available#987
dbasunag merged 3 commits intoopendatahub-io:mainfrom
dbasunag:minor_bug

Conversation

@dbasunag
Copy link
Copy Markdown
Collaborator

@dbasunag dbasunag commented Jan 6, 2026

Description

How Has This Been Tested?

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

  • Tests
    • Added an extra synchronization step in model catalog tests after configuration updates.
    • This duplicate readiness check reduces intermittent test failures and improves overall test stability and consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 6, 2026

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

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 6, 2026

📝 Walkthrough

Walkthrough

An extra call to wait_for_model_catalog_api was added to the updated_catalog_config_map test fixture in conftest.py, invoked after the existing with ResourceEditor patch block, duplicating the readiness check for the model catalog API before the fixture proceeds.

Changes

Cohort / File(s) Summary
Test fixture update
tests/model_registry/model_catalog/conftest.py
Added a second wait_for_model_catalog_api call (using model_registry_rest_headers) after the with ResourceEditor patch block inside updated_catalog_config_map, duplicating the API readiness wait.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ 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 title accurately describes the main change: adding a wait step for the catalog API to become available in updated_catalog_config_map, which matches the code modification.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09dafb7 and 970384c.

📒 Files selected for processing (1)
  • tests/model_registry/model_catalog/conftest.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/model_registry/model_catalog/conftest.py

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
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI Agents
In @tests/model_registry/model_catalog/conftest.py:
- Line 125: The post-cleanup API wait is missing in two fixtures; update
update_configmap_data_add_model and updated_catalog_config_map_scope_function to
call wait_for_model_catalog_api(url=model_catalog_rest_url[0],
headers=model_registry_rest_headers) immediately after the ResourceEditor
context exits (just like updated_catalog_config_map does), keeping the existing
is_model_catalog_ready checks intact; this ensures the same synchronization
pattern used inside the context is also applied after configmap patches are
reverted.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ce4e49 and 09dafb7.

📒 Files selected for processing (1)
  • tests/model_registry/model_catalog/conftest.py
🧰 Additional context used
🧬 Code graph analysis (1)
tests/model_registry/model_catalog/conftest.py (2)
tests/model_registry/utils.py (1)
  • wait_for_model_catalog_api (718-727)
tests/model_registry/conftest.py (2)
  • model_catalog_rest_url (641-650)
  • model_registry_rest_headers (325-326)

Comment thread tests/model_registry/model_catalog/conftest.py
Copy link
Copy Markdown
Contributor

@fege fege left a comment

Choose a reason for hiding this comment

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

/lgtm

@dbasunag dbasunag enabled auto-merge (squash) January 7, 2026 10:53
@dbasunag dbasunag merged commit 5287612 into opendatahub-io:main Jan 7, 2026
8 checks passed
@dbasunag dbasunag deleted the minor_bug branch January 7, 2026 11:15
@dbasunag
Copy link
Copy Markdown
Collaborator Author

dbasunag commented Jan 7, 2026

/cherry-pick 3.2

@rhods-ci-bot
Copy link
Copy Markdown
Contributor

Cherry pick action created PR #988 successfully 🎉!
See: https://github.com/opendatahub-io/opendatahub-tests/actions/runs/20779650856

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 7, 2026

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

dbasunag added a commit that referenced this pull request Jan 7, 2026
…available (#987) (#988)

Co-authored-by: Debarati Basu-Nag <dbasunag@redhat.com>
mwaykole pushed a commit to mwaykole/opendatahub-tests that referenced this pull request Jan 23, 2026
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.

4 participants