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 kfp pipelines testing in github workflow. #611

Merged
merged 6 commits into from
Sep 30, 2024
Merged

Conversation

revit13
Copy link
Collaborator

@revit13 revit13 commented Sep 22, 2024

Why are these changes needed?

Fix kfp pipelines testing in github workflow.

Copy link
Member

@daw3rd daw3rd left a comment

Choose a reason for hiding this comment

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

Also, because we now have 2 workflows per transform maybe we shoulld change the naming convention. For example, universal-noop-test-kfp.yml and universal-noop-test.yml.?

.github/workflows/test-kfp-transform.template Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

why is this file being added? I don't see the code/inputcode2parquet transform in dev

.github/workflows/test1.yml Outdated Show resolved Hide resolved
@revit13 revit13 force-pushed the kfp17 branch 3 times, most recently from 9f8eb4b to 429d1b3 Compare September 23, 2024 13:29
Signed-off-by: Revital Sur <eres@il.ibm.com>
Signed-off-by: Revital Sur <eres@il.ibm.com>
@revit13 revit13 force-pushed the kfp17 branch 2 times, most recently from 3879aa1 to b7e5d0d Compare September 25, 2024 19:44
@revit13 revit13 requested review from daw3rd and roytman September 25, 2024 19:44
Signed-off-by: Revital Sur <eres@il.ibm.com>
Signed-off-by: Revital Sur <eres@il.ibm.com>
.github/workflows/Makefile Outdated Show resolved Hide resolved
data-processing-lib/ray/README.md Outdated Show resolved Hide resolved
kfp/kfp_ray_components/README.md Outdated Show resolved Hide resolved
Copy link
Member

@roytman roytman left a comment

Choose a reason for hiding this comment

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

LGTM

- "releases/**"
paths:
- "@TARGET_TRANSFORM_DIR@/**"
- "!data-processing-lib/**" # This is tested in separate workflow
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe. you needs this NOTs on data-processing-lib or kfp

- "!kfp/**.md"
- "!kfp/**/doc/**"
- "data-processing-lib/**"
- "!data-processing-lib/**/test/**"
Copy link
Member

Choose a reason for hiding this comment

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

Should not need this NOTs on data-processing-lib

Copy link
Member

Choose a reason for hiding this comment

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

oops my bad. I did not see the inclusion on data-processing-lib/**

3. When the shared kfp components changes, test a randomly selected transform test
(We would like to avoid running all transform kfp tests in one PR)
3. When the shared kfp components changes or core dpk lib components files changes,
test a randomly selected transform test. Otherwise run kfp test for the changed transforms.
4. Extra credit: If .md or other non-code changes are made, run no tests.
Copy link
Member

Choose a reason for hiding this comment

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

i think this number 4 can be removed.

Signed-off-by: Revital Sur <eres@il.ibm.com>
Signed-off-by: Revital Sur <eres@il.ibm.com>
@daw3rd daw3rd merged commit 4b59c9b into IBM:dev Sep 30, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants