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

Add e2e test for Caikit Nlp Client #1075

Merged
merged 47 commits into from
Jan 15, 2024

Conversation

bdattoma
Copy link
Contributor

@bdattoma bdattoma commented Dec 12, 2023

This is a PR for testing https://github.com/opendatahub-io/caikit-nlp-client against models which are deployed using RHOAI single-model serving stack.

The test code was originally written in a python script in my local copy of the caikit-nlp repo for an ease and faster execution. Now I'm moving it to ods-ci.

UPDATE 21st Dec
Due to an incompatibility between the python libraries caikit-nlp-client and kfp, we cannot add the former in our poetry.toml. We have tried with engineers to relax the caikit-nlp-client dependency requirements but it breaks its functioning.
It means that I had to remove the CLI tests I previously created and replace it (check below section)

Solution

  • create a Jupyter notebook to send queries to the models
  • create a workbench
  • upload the jupyter notebook (and the generated certificates) in the workbench
  • execute the workbench
  • check there are no errors

Notes

  • some refactoring is applied to move model serving keywords into common resource files
  • python didn't get upgrade anymore as part of this PR
  • the original version of the tests in this PR can still be found on one of my branches bdattoma:caikit-py-client-pythontests

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.

@bdattoma bdattoma force-pushed the f/caikit-py-client branch 2 times, most recently from fa7d740 to 300cd5a Compare December 20, 2023 15:52
@bdattoma
Copy link
Contributor Author

this one cannot be tested on CI yet for some pkg incompatibility. Will align with @aloganat

@bdattoma bdattoma requested a review from lugi0 December 21, 2023 16:03
@bdattoma bdattoma self-assigned this Dec 21, 2023
@bdattoma
Copy link
Contributor Author

bdattoma commented Jan 9, 2024

PR validation

  1. caikit-nlp-client test suite: rhods-ci-pr-test/2296 PASS
  2. Kserve CLI smoke: rhods-ci-pr-test/2298/ PASS
  3. KServe UI smoke: rhods-ci-pr-test/2299/ PASS

@bdattoma bdattoma added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins labels Jan 9, 2024
Copy link

sonarcloud bot commented Jan 15, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

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

See analysis details on SonarCloud

@bdattoma bdattoma merged commit 1c294fb into red-hat-data-services:master Jan 15, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants