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.

Sign up for GitHub

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

Jump to bottom

Add TGIS test for bigscience/mt0-xxl model #1244

Merged
merged 14 commits into from
Mar 5, 2024
Merged

Add TGIS test for bigscience/mt0-xxl model #1244

merged 14 commits into from
Mar 5, 2024

Conversation

bdattoma
Copy link
Contributor

@bdattoma bdattoma commented Mar 1, 2024 •
edited
Loading

Implementations:

  1. set Raw deployment mode at ISVC level
  2. implement logic to query via port-forwarding instead of Route - needs to be further extended to be able to query with service URL instead of localhost (for future PRs)
  3. add test case for bigscience/mt0-xxl (will be deprecated soon, but could be a guideline to add tests for other models)

Validation of the PR:

  • validated locally the new code + regression tests on previous TGIS suite which leveraged on Serverless deployment
  • on CI: rhods-ci-pr-test/2552 Regression passed. New test failed because of timeout. rhods-ci-pr-test/2553 passed

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 1, 2024
View reviewed changes
ods_ci/tests/Resources/Files/llm/download_model_in_pvc.yaml
- mountPath: "/mnt/models/"
name: model-volume
containers:
- name: download-model

Check warning

Code scanning / SonarCloud

CPU limits should be enforced Medium test

Specify a CPU limit for this container. See more on SonarCloud
ods_ci/tests/Resources/Files/llm/download_model_in_pvc.yaml
- mountPath: "/mnt/models/"
name: model-volume
containers:
- name: download-model

Check warning

Code scanning / SonarCloud

Memory limits should be enforced Medium test

Specify a memory limit for this container. See more on SonarCloud
ods_ci/tests/Resources/Files/llm/download_model_in_pvc.yaml
name: download-${model_name}
labels:
name: download-${model_name}
spec:

Check warning

Code scanning / SonarCloud

Service account tokens should not be mounted in pods Medium test

Set automountServiceAccountToken to false for this specification of kind Pod. See more on SonarCloud
ods_ci/tests/Resources/Files/llm/download_model_in_pvc.yaml
- mountPath: "/mnt/models/"
name: model-volume
containers:
- name: download-model

Check warning

Code scanning / SonarCloud

Storage limits should be enforced Medium test

Specify a storage limit for this container. See more on SonarCloud
@bdattoma bdattoma marked this pull request as ready for review March 4, 2024 18:54
@bdattoma bdattoma self-assigned this Mar 4, 2024
@bdattoma bdattoma added needs testing Needs to be tested in Jenkins new test New test(s) added (PR will be listed in release-notes) labels Mar 4, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 4, 2024
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Robocop found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 4, 2024

Robot Results

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

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

tarukumar
tarukumar approved these changes Mar 5, 2024
View reviewed changes
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 tarukumar merged commit 6871f5d into red-hat-data-services:master Mar 5, 2024
12 of 13 checks passed
@bdattoma bdattoma added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins labels Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwaykole mwaykole mwaykole approved these changes

@lugi0 lugi0 lugi0 approved these changes

@tarukumar tarukumar tarukumar approved these changes

Assignees

@bdattoma bdattoma

Labels
new test New test(s) added (PR will be listed in release-notes) verified This PR has been tested with Jenkins
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bdattoma @mwaykole @lugi0 @tarukumar