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

Add tensorflow-serving-api to jupyter image #358

Merged
merged 2 commits into from Mar 19, 2018
Merged

Add tensorflow-serving-api to jupyter image #358

merged 2 commits into from Mar 19, 2018

Conversation

inc0
Copy link

@inc0 inc0 commented Mar 5, 2018 •
edited by jlewi

It's really hacky way to add api to jupyter image. We should remove it
and make it properly once package becomes available for python 3.

Fixes #355

This change is Reviewable

@inc0
Copy link
Author

inc0 commented Mar 5, 2018

/assign @jlewi

jlewi
jlewi reviewed Mar 6, 2018
View reviewed changes
components/tensorflow-notebook-image/Dockerfile.cpu Outdated
@@ -1,5 +1,12 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.

# TODO(inc0): When tf-serving-api becomes available for py3, this should be
Copy link
Contributor

Choose a reason for hiding this comment

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

What about the GPU image?

Copy link
Author

Choose a reason for hiding this comment

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

We merged images together now, rebased for it

components/tensorflow-notebook-image/Dockerfile.cpu Outdated
@@ -161,6 +168,8 @@ RUN apt-get update && apt-get install -yq --no-install-recommends graphviz \
# Install Python 3 Tensorflow without GPU support
RUN pip install --quiet --no-cache-dir tf-nightly

COPY --from=tf-serving-install /usr/local/lib/python2.7/site-packages/tensorflow_serving /opt/conda/lib/python3.6/site-packages/tensorflow_serving
Copy link
Contributor

Choose a reason for hiding this comment

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

Using the 2.7 version of TF Serving API with Python 3 seems like a recipe for problems.
Do we really want to do this?

It seems like a bad idea. But since this is what the official docs say:
https://github.com/tensorflow/serving/blob/master/tensorflow_serving/g3doc/setup.md#tensorflow-serving-python-api-pip-package

I guess it might be OK. Can you please add a comment though that this is what the docs recommend?

@jlewi
Copy link
Contributor

jlewi commented Mar 17, 2018

/lgtm
/approve

@jlewi
Copy link
Contributor

jlewi commented Mar 17, 2018

Test failure looks unrelated to this PR

generate objects for namespace : unable to read /src/kubeflow/kubeflow/testing/workflows/environments/kubeflow-presubmit-kubeflow-e2e-358-a3e3ed1-672-27bc/main.jsonnet: RUNTIME ERROR: Couldn't open import "k.libsonnet": No match locally or in the Jsonnet library paths.
-------------------------------------------------
	/src/kubeflow/kubeflow/testing/workflows/components/workflows.jsonnet:3:11-30	thunk <k> from <$>

local k = import "k.libsonnet";

@jlewi jlewi mentioned this pull request Mar 17, 2018
Closed
@jlewi
Copy link
Contributor

jlewi commented Mar 17, 2018

@inc0 can you try syncing? I suspect you just need to pull in changes to the tests.

@inc0
Add tensorflow-serving-api to jupyter image
446207e 
It's really hacky way to add api to jupyter image. We should remove it
and make it properly once package becomes available for python 3.

Fixes kubeflow#355
@inc0
remove redundant dir creation
789793f
@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 19, 2018
@jlewi
Copy link
Contributor

jlewi commented Mar 19, 2018

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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 9fc3b7c into kubeflow:master Mar 19, 2018
yanniszark pushed a commit to arrikto/kubeflow that referenced this pull request Nov 1, 2019
@k8s-ci-robot
fix for backtick escape error (kubeflow#358)
d47d06a 
* fix for backtick escape error

* rebased from upstream/master
elenzio9 pushed a commit to arrikto/kubeflow that referenced this pull request Oct 31, 2022
@thesuperzapper
create wg-notebook-leads group (kubeflow#358)
5819998
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlewi jlewi jlewi left review comments

@zjj2wry zjj2wry Awaiting requested review from zjj2wry

Assignees

@jlewi jlewi

Labels
approved lgtm size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@inc0 @jlewi @k8s-ci-robot