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

Release b0147bdbed9f25212408e0468a475289e80e0406 #1238

Merged
merged 3 commits into from
Apr 26, 2019
Merged

Release b0147bdbed9f25212408e0468a475289e80e0406 #1238

merged 3 commits into from
Apr 26, 2019

Conversation

Ark-kun
Copy link
Contributor

@Ark-kun Ark-kun commented Apr 26, 2019 •
edited by jlewi
Loading

This change is Reviewable

animeshsingh
animeshsingh approved these changes Apr 26, 2019
View reviewed changes
Copy link
Contributor

@animeshsingh animeshsingh left a comment

Choose a reason for hiding this comment

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

/lgtm
We should look at a better strategy for this though. Either use constant URLs which get mapped by backend registry, or define global variables which can be referenced everywhere. Current approach is very error prone

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Apr 26, 2019

/test kubeflow-pipeline-e2e-test

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Apr 26, 2019

We should look at a better strategy for this though.

I'll implement the better way soon. The images/components won't be updated unless the code was changed.

Either use constant URLs which get mapped by backend registry, or define global variables which can be referenced everywhere.
Current approach is very error prone

Current approach provides a choice between exact reproducibility and using the latest version.

IronPan
IronPan reviewed Apr 26, 2019
View reviewed changes
components/dataflow/predict/component.yaml
@@ -15,7 +15,7 @@ outputs:
- {name: Predictions dir, type: GCSPath, description: 'GCS or local directory.'} #Will contain prediction_results-* and schema.json files; TODO: Split outputs and replace dir with single file # type: {GCSPath: {path_type: Directory}}
implementation:
container:
image: gcr.io/ml-pipeline/ml-pipeline-dataflow-tf-predict:e20fad3e161e88226c83437271adb063221459b9
image: gcr.io/ml-pipeline/ml-pipeline-dataflow-tf-predict:b0147bdbed9f25212408e0468a475289e80e0406
Copy link
Member

Choose a reason for hiding this comment

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

why this sha instead of e8524ee?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to release component images as from b0147bd (as the title indicates).
e8524ee is the commit that does this (updates images to b0147bd).
A commit cannot reference itself >_<

P.S. This PR has 3 commits.

@IronPan
Copy link
Member

IronPan commented Apr 26, 2019

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: IronPan

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

1 similar comment
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: IronPan

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

@k8s-ci-robot k8s-ci-robot merged commit 7228ba1 into master Apr 26, 2019
@Ark-kun Ark-kun deleted the release-b0147bdbed9f25212408e0468a475289e80e0406 branch April 26, 2019 17:54
magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this pull request Oct 22, 2023
@yuzisun
Update and fix kafka example for v1beta1 (kubeflow#1238)
92c5d2c 
* Update kafka example with latest knative version

* Set address url according to inference protocol

* Add mms check for addressable url

* Fix mnist kafka event transformer

* Address comments

* Add step to upload the model
magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this pull request Oct 22, 2023
@drewbutlerbb4 @Tomcli @animeshsingh
Adversarial robustness explainer (kubeflow#1244)
5f35163 
* Add SquareAttack to artserver and rework example predictor

* Refactor mnist example

* Update example script

* Init readme

* Remove unnecessary entrypoint arguments

* add art explainer api and sdk code gen

* add test overlay

* update art.yaml to v1beta1 art spec

* Update art predictor and docs

* add art explainer e2e test

* Update e2e test and predictor image

* Update art e2e test

* Apply suggestions from code review

Co-authored-by: Animesh Singh <[email protected]>

* update eventing addressable url from kubeflow#1238

* regenerate sdk with new predictor protocols

* update e2e test

* fix typo

* fix typo

* regenerate sdk spec

* update crd with makefile controller-gen

* reduce art image size

* Apply suggestions from code review

Co-authored-by: Animesh Singh <[email protected]>

* configure manifests to not directly apply crd

* add namespace resource to the crd patch

* add missing version to test

Co-authored-by: Tommy Li <[email protected]>
Co-authored-by: Animesh Singh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IronPan IronPan IronPan left review comments

@animeshsingh animeshsingh animeshsingh approved these changes

@gaoning777 gaoning777 Awaiting requested review from gaoning777

@hongye-sun hongye-sun Awaiting requested review from hongye-sun

Assignees

@IronPan IronPan

@animeshsingh animeshsingh

Labels
approved area/engprod lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ark-kun @IronPan @k8s-ci-robot @animeshsingh