Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update runtime yamls #1182

Merged
merged 5 commits into from
Feb 15, 2024
Merged

Conversation

bdattoma
Copy link
Contributor

@bdattoma bdattoma commented Feb 6, 2024

Aligning our caikit+tgis runtime yaml with https://github.com/opendatahub-io/caikit-tgis-serving/blob/main/demo/kserve/custom-manifests/caikit/caikit-tgis/caikit-tgis-servingruntime-grpc.yaml

UPDATE: there will be a slight additional update to the manifest opendatahub-io/caikit-tgis-serving#217. So I would wait merging this test PR until we include the new changes

@bdattoma bdattoma added needs testing Needs to be tested in Jenkins enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) labels Feb 6, 2024
@bdattoma bdattoma self-assigned this Feb 6, 2024
@bdattoma
Copy link
Contributor Author

bdattoma commented Feb 6, 2024

PR validation (OLD):

  1. Basic Caikit+TGIS grpc and http: rhods-ci-pr-test/2442 PASS
  2. Misc Caikit+TGIS (grpc) rhods-ci-pr-test/2447 PASS - there is one failure because I didn't exclude the GPU test and it ran on a cluster without GPUs
  3. Caikit+TGIS with caikit-nlp-client: rhods-ci-pr-test/2448/ PASS

Copy link
Contributor

github-actions bot commented Feb 6, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
401 0 0 401 100

tarukumar
tarukumar previously approved these changes Feb 7, 2024
Copy link
Contributor

@tarukumar tarukumar left a comment

Choose a reason for hiding this comment

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

/lgtm

@tarukumar
Copy link
Contributor

you need to update the link in description properly :)

@bdattoma bdattoma added the do not merge Do not merge this yet please label Feb 7, 2024
@bdattoma bdattoma added verified This PR has been tested with Jenkins needs testing Needs to be tested in Jenkins do not merge Do not merge this yet please and removed do not merge Do not merge this yet please needs testing Needs to be tested in Jenkins verified This PR has been tested with Jenkins labels Feb 9, 2024
@bdattoma bdattoma changed the title Update caikit+tgis runtime yaml Update runtime yamls Feb 13, 2024
@bdattoma
Copy link
Contributor Author

bdattoma commented Feb 13, 2024

PR Validation after image URL changes:

  • Sanity run for Caikit+TGIS and TGIS standalone: rhods-ci-pr-test/2501 PASS

@bdattoma bdattoma removed the do not merge Do not merge this yet please label Feb 14, 2024
@bdattoma bdattoma requested a review from tarukumar February 15, 2024 09:52
@bdattoma bdattoma added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins labels Feb 15, 2024
@bdattoma bdattoma requested a review from lugi0 February 15, 2024 09:53
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

# resources: # configure as required
# requests:
# cpu: 8
# memory: 16Gi
Copy link
Member

Choose a reason for hiding this comment

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

Are these two commented blocks for resources in these two files necessary to keep?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i want to keep the manifest equal to the one published upstream, apart from image url which must change

Copy link
Member

@jstourac jstourac left a comment

Choose a reason for hiding this comment

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

one comment, but it's optional, lgtm

don't forget to squash 😄

@bdattoma bdattoma merged commit 80fd3d9 into red-hat-data-services:master Feb 15, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) verified This PR has been tested with Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants