Skip to content

Use latest aync job image published#598

Merged
dbasunag merged 2 commits intoopendatahub-io:mainfrom
fege:fix_job_image
Sep 9, 2025
Merged

Use latest aync job image published#598
dbasunag merged 2 commits intoopendatahub-io:mainfrom
fege:fix_job_image

Conversation

@fege
Copy link
Copy Markdown
Contributor

@fege fege commented Sep 9, 2025

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
    • Updated the asynchronous upload job’s container image used in test runs to version v0.3.0.
    • Ensures test environment aligns with the latest runtime, improving reliability and consistency across CI.
    • No changes to application behavior or user-facing features.

@fege fege requested review from dbasunag and lugi0 as code owners September 9, 2025 13:07
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 9, 2025

📝 Walkthrough

Walkthrough

Updated the ASYNC_UPLOAD_IMAGE constant in tests/model_registry/async_job/constants.py from v0.2.22 to v0.3.0. No other constants, logic, or signatures changed.

Changes

Cohort / File(s) Summary of Changes
Async job test constants
tests/model_registry/async_job/constants.py
Bumped ASYNC_UPLOAD_IMAGE tag from v0.2.22 to v0.3.0; no other edits

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Pre-merge checks (2 passed, 1 inconclusive)

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description Check ❓ Inconclusive The PR description currently comprises only a template with empty sections for Description and How Has This Been Tested, offering no substantive detail about the change to the ASYNC_UPLOAD_IMAGE constant or its testing, which makes it too vague to support a clear review. Populate the Description section with details about updating the ASYNC_UPLOAD_IMAGE constant from v0.2.22 to v0.3.0 and complete the How Has This Been Tested section with information on testing steps and environments.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The current title “Use latest aync job image published” directly addresses the main change of updating the async job Docker image and therefore reflects the primary change in the PR, but it contains a spelling error (“aync” vs. “async”) and could be more specific by including the actual version number.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ 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

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

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 9, 2025

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

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

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: 0

🧹 Nitpick comments (1)
tests/model_registry/async_job/constants.py (1)

3-3: LGTM on bump to v0.3.0.

The image tag update looks fine. Consider pinning to a digest for deterministic CI and resilience against tag expiry/retagging on Quay. Also optionally allow an env override to reduce future churn.

Apply one of these diffs:

Option A — pin to digest (replace after verifying):

-ASYNC_UPLOAD_IMAGE = "quay.io/opendatahub/model-registry-job-async-upload:v0.3.0"
+ASYNC_UPLOAD_IMAGE = "quay.io/opendatahub/model-registry-job-async-upload@sha256:<DIGEST>"

Option B — keep tag but make it overridable:

+import os
-ASYNC_UPLOAD_IMAGE = "quay.io/opendatahub/model-registry-job-async-upload:v0.3.0"
+ASYNC_UPLOAD_IMAGE = os.getenv(
+    "ASYNC_UPLOAD_IMAGE",
+    "quay.io/opendatahub/model-registry-job-async-upload:v0.3.0",
+)
📜 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 5eee07b and a180f64.

📒 Files selected for processing (1)
  • tests/model_registry/async_job/constants.py (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#487
File: tests/model_registry/async_job/utils.py:14-32
Timestamp: 2025-08-08T16:00:51.421Z
Learning: In opendatahub-io/opendatahub-tests, for tests under tests/model_registry/async_job, keep get_latest_job_pod (tests/model_registry/async_job/utils.py) simple: sorting by creationTimestamp string and selecting pods via label_selector="job-name=<job.name>" is acceptable; do not add datetime parsing or controller-uid filtering unless a concrete failure arises (per guidance from user lugi0).
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#487
File: tests/model_registry/async_job/test_async_upload_e2e.py:344-356
Timestamp: 2025-08-08T15:59:16.170Z
Learning: In opendatahub-io/opendatahub-tests, tests/model_registry/async_job/test_async_upload_e2e.py’s test_end_to_end_async_upload_job intentionally expects an OCI image index (application/vnd.oci.image.index.v1+json) from pull_manifest_from_oci_registry and asserts on the "manifests" array. This format has been verified by the author (lugi0); no change to Accept headers or assertions is required.
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#487
File: tests/model_registry/async_job/constants.py:23-31
Timestamp: 2025-08-08T15:58:03.524Z
Learning: In opendatahub-io/opendatahub-tests, for tests under tests/model_registry/async_job (e.g., constants.py and related fixtures), runs start from a clean, known Model Registry state. Therefore, using static MODEL_ID, MODEL_VERSION_ID, and MODEL_ARTIFACT_ID values in MODEL_SYNC_CONFIG is intentional and acceptable; dynamic ID injection is not required for these async job tests (per guidance from user lugi0).
📚 Learning: 2025-08-08T15:59:16.170Z
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#487
File: tests/model_registry/async_job/test_async_upload_e2e.py:344-356
Timestamp: 2025-08-08T15:59:16.170Z
Learning: In opendatahub-io/opendatahub-tests, tests/model_registry/async_job/test_async_upload_e2e.py’s test_end_to_end_async_upload_job intentionally expects an OCI image index (application/vnd.oci.image.index.v1+json) from pull_manifest_from_oci_registry and asserts on the "manifests" array. This format has been verified by the author (lugi0); no change to Accept headers or assertions is required.

Applied to files:

  • tests/model_registry/async_job/constants.py
📚 Learning: 2025-08-08T15:58:03.524Z
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#487
File: tests/model_registry/async_job/constants.py:23-31
Timestamp: 2025-08-08T15:58:03.524Z
Learning: In opendatahub-io/opendatahub-tests, for tests under tests/model_registry/async_job (e.g., constants.py and related fixtures), runs start from a clean, known Model Registry state. Therefore, using static MODEL_ID, MODEL_VERSION_ID, and MODEL_ARTIFACT_ID values in MODEL_SYNC_CONFIG is intentional and acceptable; dynamic ID injection is not required for these async job tests (per guidance from user lugi0).

Applied to files:

  • tests/model_registry/async_job/constants.py
🔇 Additional comments (1)
tests/model_registry/async_job/constants.py (1)

3-3: Pin ASYNC_UPLOAD_IMAGE to an immutable digest
v0.3.0 is present in Quay, but the digest lookup script returned no value. Manually retrieve the Docker-Content-Digest for quay.io/opendatahub/model-registry-job-async-upload:v0.3.0 (for example, via curl -I -H 'Accept: application/vnd.docker.distribution.manifest.v2+json' https://quay.io/v2/opendatahub/model-registry-job-async-upload/manifests/v0.3.0 | awk -F': ' '/^Docker-Content-Digest:/ {print $2}') and update ASYNC_UPLOAD_IMAGE to use the @sha256:<digest> reference.

@dbasunag dbasunag merged commit b9a7046 into opendatahub-io:main Sep 9, 2025
11 checks passed
@fege fege deleted the fix_job_image branch September 9, 2025 13:19
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 9, 2025

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

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