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

Create pipeline run with different user #1628

Merged
merged 1 commit into from
Aug 31, 2024
Merged

Create pipeline run with different user #1628

merged 1 commit into from
Aug 31, 2024

Conversation

diegolovison
Copy link
Contributor

@diegolovison diegolovison commented Jul 10, 2024 •
edited
Loading

Tested with RHOAI 2.13 - Nightly and all tests passed

sh run_robot_test.sh --set-urls-variables true --extra-robot-args '-L DEBUG' --test-case 'tests/Tests/1100__data_science_pipelines/1100__data-science-pipelines-kfp.robot' --skip-oclogin

Verify Ods Users Can Create And Run A Data Science Pipeline With R... | PASS |
------------------------------------------------------------------------------
Data-Science-Pipelines-Kfp :: Test suite for OpenShift Pipeline us... | PASS |
3 tests, 3 passed, 0 failed
==============================================================================

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 10, 2024 •
edited
Loading

Robot Results

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

@diegolovison diegolovison force-pushed the draft_permission branch 2 times, most recently from 4434c3d to d6ce16b Compare July 10, 2024 23:09
@diegolovison diegolovison marked this pull request as ready for review July 10, 2024 23:10
bdattoma
bdattoma previously approved these changes Jul 11, 2024
View reviewed changes
jgarciao
jgarciao requested changes Jul 16, 2024
View reviewed changes
ods_ci/tests/Tests/1100__data_science_pipelines/1100__data-science-pipelines-kfp.robot Outdated
Install DataSciencePipelinesApplication CR ${project}
${status} Login And Wait Dsp Route ${username} ${password} ${project}
${status} Login And Wait Dsp Route ${TEST_USER.USERNAME} ${TEST_USER.PASSWORD} ${project}
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, mixing global variables and parameters here for username and password is confusing, it looks like an error.

I think it would be better to add new parameters like ${username_create_run} and ${password_create_run}, or just not using the keyword End To End Pipeline Workflow Using Kfp for this specific test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it is a static value. Static values are for this purpose. I think having a new variable just to have the value of a static value is not good practice, IMHO.

Copy link
Contributor

Choose a reason for hiding this comment

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

The current approach looks good to me. Thanks Diego!

@diegolovison diegolovison force-pushed the draft_permission branch 2 times, most recently from 4b773c6 to 3fee5d2 Compare August 30, 2024 14:19
jgarciao
jgarciao previously approved these changes Aug 30, 2024
View reviewed changes
jstourac
jstourac previously approved these changes Aug 30, 2024
View reviewed changes
@diegolovison diegolovison added needs testing Needs to be tested in Jenkins do not merge Do not merge this yet please and removed needs testing Needs to be tested in Jenkins do not merge Do not merge this yet please labels Aug 30, 2024
@diegolovison diegolovison dismissed stale reviews from jstourac and jgarciao via 95f8377 August 30, 2024 18:18
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@jgarciao jgarciao self-requested a review August 31, 2024 12:34
@jgarciao jgarciao merged commit 7759187 into red-hat-data-services:master Aug 31, 2024
10 checks passed
jgarciao pushed a commit to jgarciao/ods-ci that referenced this pull request Sep 1, 2024
@diegolovison @jgarciao
Create pipeline run with different user (red-hat-data-services#1628)
8f47fe6
tonyxrmdavidson pushed a commit to tonyxrmdavidson/ods-ci that referenced this pull request Sep 4, 2024
@diegolovison @tonyxrmdavidson
Create pipeline run with different user (red-hat-data-services#1628)
fb614b2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgarciao jgarciao jgarciao approved these changes

@jiridanek jiridanek jiridanek approved these changes

@jstourac jstourac jstourac left review comments

@bdattoma bdattoma bdattoma left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@diegolovison @jgarciao @jiridanek @jstourac @bdattoma