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

Fix 'Clean All Models Of Current User' KW #1203

Merged
merged 3 commits into from
Feb 18, 2024

Conversation

manosnoam
Copy link
Contributor

In case the current user does not have permissions to view other DS projects, then the Model Serving page does not include "select a project" link, which causes the 'Clean All Models Of Current User' KW to fail.

The fix includes:

  • Skip cleanup if no Data Sciense projects exist
  • 'Set Up Project' SETUP KW continues if cleanup failed
  • 'Set Up Project' SETUP KW creates DS project only if doesn't exist yet

image

In case the current user does not have permissions to view other DS
projects, then the Model Serving page does not include "select a project"
link, which causes the 'Clean All Models Of Current User' KW to fail.

The fix includes:
- Skip cleanup if no Data Sciense projects exist
- 'Set Up Project' SETUP KW continues if cleanup failed
- 'Set Up Project' SETUP KW creates DS project only if doesn't exist yet

Signed-off-by: manosnoam <[email protected]>
END
Open Data Science Projects Home Page
Wait For RHODS Dashboard To Load wait_for_cards=${FALSE} expected_page=Data Science Projects
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

@manosnoam manosnoam Feb 15, 2024

Choose a reason for hiding this comment

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

Because Open Data Science Projects Home Page already does it :)

And regarding
Create Data Science Project title=${namespace} description=kserve test project existing_project=${TRUE}
I did it since that's what the KW docstring says:

Set Up Project
[Documentation] Creates the DS Project (if not exists)

Copy link
Contributor

Choose a reason for hiding this comment

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

Because Open Data Science Projects Home Page already does it :)

Ok

I wasn't questioning the create DS project part :)

Copy link
Contributor

github-actions bot commented Feb 15, 2024

Robot Results

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

@manosnoam manosnoam added verified This PR has been tested with Jenkins enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) labels Feb 15, 2024
@manosnoam manosnoam requested a review from bdattoma February 15, 2024 15:31
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

@manosnoam
Copy link
Contributor Author

@bdattoma I've also verify the fix by running another suite that uses 'Clean All Models Of Current User' KW at suite's Setup and Teardown, and it works as expected:
image

The failed test (Verify Secure Model Can Be Deployed In Same Project) is due to a recent product bug: https://issues.redhat.com/browse/RHOAIENG-2759

@manosnoam manosnoam requested a review from lugi0 February 16, 2024 09:50
Copy link
Contributor

@lugi0 lugi0 left a comment

Choose a reason for hiding this comment

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

LGTM but please test against the kserve UI suites before merging

END
Open Data Science Projects Home Page
Wait For RHODS Dashboard To Load wait_for_cards=${FALSE} expected_page=Data Science Projects
Create Data Science Project title=${namespace} description=kserve test project
Create Data Science Project title=${namespace} description=kserve test project existing_project=${TRUE}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only line I'm not sure about, specifically existing_project=${TRUE}.
Could you test the change against 422__model_serving_caikit_py_client.robot, 422__model_serving_llm_other_runtimes_UI.robot and 422__model_serving_llm_UI.robot?

@manosnoam manosnoam merged commit ef1fac2 into red-hat-data-services:master Feb 18, 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