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

feat(Tekton-compiler): Add env vars in kfp-driver to parametrize Object storage host and port #1378

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

rimolive
Copy link
Member

Which issue is resolved by this Pull Request:
N/A

Description of your changes:
In our multi-stack architecture for Kubeflow Pipelines, we need the kfp-driver to support parametrization of the MLMD location.

Environment tested:

  • Python Version (use python --version): Python 3.10.12
  • Tekton Version (use tkn version): N/A
  • Kubernetes Version (use kubectl version): v1.25.2
  • OS (e.g. from /etc/os-release): Fedora 37

Checklist:

@google-oss-prow google-oss-prow bot requested review from Tomcli and yhwang October 16, 2023 15:42
@google-oss-prow google-oss-prow bot added size/S and removed size/M labels Oct 16, 2023
@Tomcli
Copy link
Member

Tomcli commented Oct 16, 2023

@rimolive since you changed the compiled Tekton yaml output, you also need to update the Tekton test cases over here in order to pass the unit tests.
https://github.com/kubeflow/kfp-tekton/tree/v2-integration/backend/src/v2/compiler/tektoncompiler/testdata

This is part of the unit test over here https://github.com/kubeflow/kfp-tekton/blob/v2-integration/backend/src/v2/compiler/tektoncompiler/tekton_test.go#L90

@google-oss-prow google-oss-prow bot added size/M and removed size/S labels Oct 16, 2023
@rimolive
Copy link
Member Author

/retest

@Tomcli
Copy link
Member

Tomcli commented Oct 16, 2023

/approve
/lgtm

I need to double check the license on some of the images. Tekton 0.50.2 just released today with our regression and security fixes so we are going to update the Tekton client and licenses.

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rimolive, Tomcli

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Tomcli
Copy link
Member

Tomcli commented Oct 16, 2023

The github ci needs to be updated with the new examples in kfp 2.0.2. I have a PR to address it.
#1380

@Tomcli
Copy link
Member

Tomcli commented Oct 16, 2023

I need to double check the license on some of the images.

I double checked the images and seems like just github action wasn't stable on pulling go package license names. On IBM Cloud our CI is more stable and able to build all the images without any problem.

@Tomcli Tomcli changed the title feat(kfp-driver): Add env vars in kfp-driver to parametrize MLMD host and port feat(Tekton-compiler): Add env vars in kfp-driver to parametrize MLMD host and port Oct 18, 2023
@Tomcli Tomcli changed the title feat(Tekton-compiler): Add env vars in kfp-driver to parametrize MLMD host and port feat(Tekton-compiler): Add env vars in kfp-driver to parametrize Object storage host and port Oct 18, 2023
@google-oss-prow google-oss-prow bot removed the lgtm label Oct 18, 2023
@Tomcli
Copy link
Member

Tomcli commented Oct 18, 2023

#1380 addressed the ci issue and I moved some image builds to the master branch because some image builds were reaching github rate limiting issues when checking licenses.

@Tomcli
Copy link
Member

Tomcli commented Oct 19, 2023

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Oct 19, 2023
@google-oss-prow google-oss-prow bot merged commit 7a14e32 into kubeflow:v2-integration Oct 19, 2023
3 checks passed
yhwang pushed a commit to yhwang/kfp-tekton that referenced this pull request Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants