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

Update for Kubeflow 1.4 release #7

Merged
merged 7 commits into from
Sep 17, 2021
Merged

Update for Kubeflow 1.4 release #7

merged 7 commits into from
Sep 17, 2021

Conversation

ca-scribner
Copy link
Contributor

  • updates charm to kubeflow 1.4
  • adds pytest operator/refactors CI

Comment on lines +1 to +11
#!/usr/bin/env bash
set -eux
MODEL=$1
echo "MODEL=$MODEL"
wget https://raw.githubusercontent.com/google/ml-metadata/master/ml_metadata/proto/metadata_store.proto
wget https://raw.githubusercontent.com/google/ml-metadata/master/ml_metadata/proto/metadata_store_service.proto
wget -O- https://github.com/fullstorydev/grpcurl/releases/download/v1.8.0/grpcurl_1.8.0_linux_x86_64.tar.gz | tar -xzv
mkdir -p ml_metadata/proto/
mv metadata_store.proto ml_metadata/proto/
SERVICE=$(kubectl get services/mlmd -n $MODEL -oyaml | yq e .spec.clusterIP -)
./grpcurl -v --proto=metadata_store_service.proto --plaintext $SERVICE:8080 ml_metadata.MetadataStoreService/GetArtifacts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I python-ify this? The kubectl call could be the python k8s sdk, wgets could be in python, then grpcurl could be a subprocess.run (or maybe connect some other way)

Alternatively, we could use the mlmd python api and just call to it using the native ways in python

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(or should we table this for future and prioritize the 1.4 release first)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, this seems like it should be a pytest fixture that does all the setup, and returns a MetadataStore (judging by my reading of the docs) object . I think it's also fine to punt on that for now and focus on 1.4.

@ca-scribner ca-scribner marked this pull request as ready for review September 16, 2021 13:57
@ca-scribner ca-scribner requested a review from a team as a code owner September 16, 2021 13:57
.github/workflows/integrate.yaml Show resolved Hide resolved
.github/workflows/integrate.yaml Outdated Show resolved Hide resolved
.github/workflows/integrate.yaml Outdated Show resolved Hide resolved
Comment on lines +1 to +11
#!/usr/bin/env bash
set -eux
MODEL=$1
echo "MODEL=$MODEL"
wget https://raw.githubusercontent.com/google/ml-metadata/master/ml_metadata/proto/metadata_store.proto
wget https://raw.githubusercontent.com/google/ml-metadata/master/ml_metadata/proto/metadata_store_service.proto
wget -O- https://github.com/fullstorydev/grpcurl/releases/download/v1.8.0/grpcurl_1.8.0_linux_x86_64.tar.gz | tar -xzv
mkdir -p ml_metadata/proto/
mv metadata_store.proto ml_metadata/proto/
SERVICE=$(kubectl get services/mlmd -n $MODEL -oyaml | yq e .spec.clusterIP -)
./grpcurl -v --proto=metadata_store_service.proto --plaintext $SERVICE:8080 ml_metadata.MetadataStoreService/GetArtifacts
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, this seems like it should be a pytest fixture that does all the setup, and returns a MetadataStore (judging by my reading of the docs) object . I think it's also fine to punt on that for now and focus on 1.4.

test-integration-requirements.txt Outdated Show resolved Hide resolved
@ca-scribner ca-scribner merged commit c97201e into master Sep 17, 2021
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.

2 participants