opendatahub-io / ai-edge

ODH integration with AI at the Edge usecases
Apache License 2.0
10 stars 19 forks source link

RHOAIENG-9803-Create a sample Git fetch pipeline in manifests #284

Closed biswassri closed 2 months ago

biswassri commented 3 months ago

Description

Replaces #279 A pipeline that does the same as the MLops pipeline but now with just the git fetch task as the fetch method. This change also removes the aiedge-e2e-pipeline from the kustomization file. So the files remains but the pipeline is not gonna get created.

How Has This Been Tested?

oc apply -k manifests/ 
oc create -f pipelines/tekton/aiedge-e2e/example-pipelineruns/git-fetch.tensorflow-housing.pipelinerun.yaml 

Merge criteria:

aspanner commented 3 months ago

The description and parameter name doesn't seem to match - can you make this clearer whether the full path or just the model name is needed? name: model-name type: string description: Name of the directory where the model files are stored

biswassri commented 3 months ago

Just that one issue of the path and also AFAIK we can delete the old pipeline as part of this PR aswell

@jackdelahunt When you old pipeline do you mean the ai-edge-e2e pipeline or the pipeline files under tekton dir? So I removed the ai-edge-e2e pipeline from Kustomizaton file, so that it's not created as a pipeline but there for reeference. But I am okay to delete that as well.

jackdelahunt commented 3 months ago

@jackdelahunt When you old pipeline do you mean the ai-edge-e2e pipeline or the pipeline files under tekton dir? So I removed the ai-edge-e2e pipeline from Kustomizaton file, so that it's not created as a pipeline but there for reeference. But I am okay to delete that as well.

Yes the ai-edge-e2e, we can delete it outright instead of just from kustomize

biswassri commented 3 months ago

Yes the ai-edge-e2e, we can delete it outright instead of just from kustomize

Cool! I'll do that. Also I think this needs to be the same as this right? I mean the param needs to be container-relative-path format right?

biswassri commented 2 months ago

@MarianMacik Done! I tried removing the aiedge-e2e pipeline file but somehow it's didn't get pushed in this commit.

MarianMacik commented 2 months ago

@biswassri I tried to resolve the conflict and made one more commit to make git-fetch pipeline contain the same improvements as s3-fetch pipeline. All your previous commits are there, so nothing is lost. I hope it is OK.

biswassri commented 2 months ago

/test test-ai-edge

biswassri commented 2 months ago

@MarianMacik Thanks! I was about to close out this PR but now it looks good to me.

MarianMacik commented 2 months ago

/retest (automation failure)

MarianMacik commented 2 months ago

/retest

jackdelahunt commented 2 months ago

Tested with e2e tests and passed, and changes look good to me

/approve

MarianMacik commented 2 months ago

/approve

openshift-ci[bot] commented 2 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackdelahunt, MarianMacik

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: - ~~[OWNERS](https://github.com/opendatahub-io/ai-edge/blob/main/OWNERS)~~ [MarianMacik] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
biswassri commented 2 months ago

/lgtm

openshift-ci[bot] commented 2 months ago

@biswassri: you cannot LGTM your own PR.

In response to [this](https://github.com/opendatahub-io/ai-edge/pull/284#issuecomment-2289154284): >/lgtm Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
jackdelahunt commented 2 months ago

/lgtm