Skip to content

Conversation

@dimakis
Copy link
Contributor

@dimakis dimakis commented Nov 8, 2023

Issue link

What changes have been made

Changing from tag to digest reference for raycluster image

Verification steps

Confirm that the sha matches the sha from the latest raycluster tag here

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • Testing is not required for this change

@dimakis dimakis requested review from KPostOffice and removed request for Maxusmusti and anishasthana November 8, 2023 16:38
Copy link
Contributor

@ChristianZaccaria ChristianZaccaria left a comment

Choose a reason for hiding this comment

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

Verified the SHA 👍 . The unit-tests are failing, but I think you just need to update this line on the test:

assert config.image == "quay.io/project-codeflare/ray:latest-py39-cu118"

@dimakis dimakis force-pushed the update-raycluster-tag-to-digest branch from 71e0ff1 to 4b2ff50 Compare November 8, 2023 17:01
@ChristianZaccaria
Copy link
Contributor

Do a cd tests , then black unit_test.py to fix pre-commit. That might fix unit-tests as well.

@dimakis dimakis force-pushed the update-raycluster-tag-to-digest branch from 4b2ff50 to 080c5a4 Compare November 8, 2023 17:16
@ChristianZaccaria
Copy link
Contributor

Copy link
Contributor

@tedhtchang tedhtchang left a comment

Choose a reason for hiding this comment

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

/LGTM

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 8, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tedhtchang
Once this PR has been reviewed and has the lgtm label, please ask for approval from dimakis. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tedhtchang
Copy link
Contributor

tedhtchang commented Nov 8, 2023

/LGTM cancel
May need to look into these 3 tests and re-run tests i.e.: pytest tests/unit_test.py::test_cluster_creation

FAILED tests/unit_test.py::test_cluster_creation - AssertionError: assert False
FAILED tests/unit_test.py::test_cluster_creation_no_mcad - AssertionError: assert False
FAILED tests/unit_test.py::test_cluster_creation_priority - AssertionError: assert False

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2023
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 10, 2023
@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

Instructions 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/test-infra repository.

@dimakis dimakis closed this Feb 27, 2024
@dimakis dimakis deleted the update-raycluster-tag-to-digest branch February 27, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants