Add clipper deployment support by yubozhao · Pull Request #151 · bentoml/BentoML

yubozhao · 2019-06-03T23:50:56Z

(Thanks for sending a pull request! Please make sure to read the contribution guidelines, then fill out the blanks below.)

What changes were proposed in this pull request?

Does this close any currently open issues?

How was this patch tested?

pep8speaks · 2019-06-03T23:51:20Z

Hello @yubozhao, Thanks for updating this PR.

In the file bentoml/deployment/clipper/__init__.py:

Line 68:89: E501 line too long (91 > 88 characters)

Comment last updated at 2019-07-16 22:50:54 UTC

bentoml/archive/py_module_utils.py

parano · 2019-06-04T00:00:37Z

.gitignore

 node_modules/
 yarn-error.log
 yarn.lock
+.DS_Store


nit:

# MacOS .DS_Store

parano · 2019-06-04T00:01:29Z

bentoml/archive/templates.py

 """
+
+BENTO_SERVICE_DOCKERFILE_CLIPPER_TEMPLATE = """\
+FROM clipper/python36-closure-container:0.3


add a note on why we use python36 image, even for BentoArchive built in python27

parano · 2019-06-04T00:02:16Z

bentoml/archive/templates.py

+
+ENV TINI_VERSION v0.16.1
+ADD https://github.com/krallin/tini/releases/download/${TINI_VERSION}/tini /usr/bin/tini
+RUN chmod +x /usr/bin/tini


do we need tini in this case?

na, removed it.

done

parano · 2019-06-04T00:05:00Z

bentoml/archive/templates.py

+RUN chmod +x /usr/bin/tini
+##### End installation for miniconda3
+
+RUN set -x \


move this up to the "apt-get" section above, let's not run apt-get update multiple times in one docker build

parano · 2019-06-04T00:05:57Z

bentoml/archive/templates.py

+WORKDIR /container
+
+# update conda base env
+RUN conda env update -n base -f /container/environment.yml


shouldn't this be "/container/bento/environment.yml"?

same for the requirement.txt and setup.sh file below?

yeah, those weren't done when i made the draft PR.

done now

parano · 2019-06-04T00:09:41Z

bentoml/deployment/clipper/__init__.py

+        shutil.copytree(self.archive_path, model_path)
+
+        image = build_docker_image(snapshot_path)
+        self.clipper_conn.register_application(name=self.application_name,


I would suggest leaving register_application and link_model_to_app to the user, similar to the "deploy_xxx" commands provided in clipper, it should only deploy the model, without affecting the application

parano · 2019-06-04T00:14:07Z

bentoml/deployment/clipper/__init__.py

+        return
+
+    def check_status(self):
+        if self.is_deployed:


discussed offline, remove check_status and delete here, will recommend user follow clipper convention on checking the model status and stopping running model instance

parano · 2019-06-04T00:17:14Z

bentoml/archive/archiver.py

    with open(os.path.join(path, 'Dockerfile-sagemaker'), 'w') as f:
        f.write(BENTO_SERVICE_DOCKERFILE_SAGEMAKER_TEMPLATE)
+    with open(os.path.join(path, 'Dockerfile-clipper'), 'w') as f:
+        f.write(BENTO_SERVICE_DOCKERFILE_CLIPPER_TEMPLATE)


discussed offline, we should handle this similar to DEFAULT_CLIPPER_ENTRY, generate it when calling deploy

same for BENTO_SERVICE_DOCKERFILE_SAGEMAKER_TEMPLATE above, but this can be in separate PR

parano · 2019-06-04T00:19:37Z

bentoml/deployment/clipper/__init__.py

+    """Clipper deployment for BentoML bundle
+    """
+
+    def __init__(self, clipper_conn, archive_path, api_name, slo_micros=100000):


remove slo_micros since we will not handle register application

parano · 2019-06-04T00:23:16Z

bentoml/deployment/clipper/__init__.py

+        return self.application_name
+
+
+def deploy_bentoml(clipper_conn, archive_path, api_name):


also expose the clipper "labels" here? http://clipper.ai/api/api.html#definition-Model

parano · 2019-06-04T00:28:58Z

bentoml/deployment/clipper/__init__.py

+
+def build_docker_image(snapshot_path):
+    docker_api = docker.APIClient()
+    tag = "something"


maybe use BentoArchive version for tag?

parano · 2019-06-04T00:31:59Z

bentoml/deployment/clipper/__init__.py

+        raise BentoMLException(
+            'Please specify api-name, when more than one API is present in the archive')
+    application_name = bento_service.name + '-' + api.name
+    input_type = 'strings'


Why do we still need 'handle_clipper_numbers'? looks like its just strings and bytes we are supporting?

…yment

codecov-io · 2019-07-11T23:21:39Z

Codecov Report

Merging #151 into master will decrease coverage by 2.02%.
The diff coverage is 19.57%.

@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
- Coverage   54.57%   52.55%   -2.03%     
==========================================
  Files          51       53       +2     
  Lines        2767     2940     +173     
==========================================
+ Hits         1510     1545      +35     
- Misses       1257     1395     +138

Impacted Files	Coverage Δ
...ntoml/deployment/serverless/aws_lambda_template.py	`34.09% <ø> (ø)`	⬆️
bentoml/deployment/clipper/templates.py	`0% <0%> (ø)`
bentoml/deployment/clipper/__init__.py	`0% <0%> (ø)`
bentoml/deployment/sagemaker/__init__.py	`21.98% <100%> (+1.47%)`	⬆️
bentoml/__init__.py	`100% <100%> (ø)`	⬆️
bentoml/handlers/fastai_image_handler.py	`33.6% <16.66%> (-5.53%)`	⬇️
bentoml/handlers/dataframe_handler.py	`65.35% <17.14%> (-18.35%)`	⬇️
bentoml/deployment/utils.py	`42.3% <22.22%> (-45.2%)`	⬇️
bentoml/handlers/image_handler.py	`61.11% <29.41%> (-7.39%)`	⬇️
bentoml/handlers/json_handler.py	`69.38% <38.46%> (-11.17%)`	⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4ae4e1...dea4ea7. Read the comment docs.

yubozhao · 2019-07-15T21:23:53Z

bentoml/handlers/dataframe_handler.py

+
+    def handle_clipper_strings(self, inputs, func):
+        def transform_and_predict(input_string):
+            # Assuming input string is JSON format


Maybe give user a choice between json and csv??

parano · 2019-07-16T15:11:41Z

bentoml/deployment/clipper/__init__.py

+        clipper_conn(clipper_admin.ClipperConnection): Clipper conntection instance
+        archive_path(str): Path to the bentoml service archive.
+        api_name(str): name of the api that will be running in the clipper cluster
+        input_type(str): Input type that clipper accept


add some more notes here - especially about the default value and behavior around ImageHnalder check below

parano · 2019-07-16T15:13:46Z

bentoml/deployment/clipper/__init__.py

+        raise BentoMLException(
+            "Please specify api-name, when more than one API is present in the archive"
+        )
+    application_name = generate_clipper_compatiable_string(


give the user the option to provide an application name as the function parameter?

parano · 2019-07-16T15:14:33Z

bentoml/handlers/dataframe_handler.py

-        if item.startswith('bool'):
-            return 'boolean'
-        return 'object'
+        if item.startswith("int"):


is this formatted by black? I thought it should be single quote?

Yes, those formatted by black. I will re-run it to make sure.

changed back. format script skip string normalization

parano · 2019-07-16T21:39:59Z

bentoml/service.py

+        return self.handler.handle_clipper_strings(inputs, self.func)
+
+    def handle_clipper_bytes(self, inputs):
+        return self.handler.handle_clipper_strings(inputs, self.func)


parano · 2019-07-16T21:43:42Z

bentoml/handlers/image_handler.py

+
+    def handle_clipper_strings(self, inputs, func):
+        raise RuntimeError(
+            "Image handler does not support 'strings' input_type \


typo? *ImageHandler

bentoml/handlers/dataframe_handler.py

parano · 2019-07-16T21:47:31Z

bentoml/deployment/utils.py

+                    sys.stderr.write(error["message"])
+                    raise RuntimeError("Error on build - code %s" % error["code"])
+                elif "stream" in line_payload:
+                    # TODO: move this to logger.info


try just use logger.info here? and similarly for the logging.error above?

parano · 2019-07-16T21:50:15Z

bentoml/deployment/clipper/templates.py

+# update conda and setup environment and pre-install common ML libraries to speed up docker build
+RUN conda update conda -y \
+      && conda install pip numpy scipy \
+      && pip install six bentoml


this is ok for now but we need to pin this to the version that user is using for saving the given BentoArchive

Provide `bentoml.deployment.clipper.deploy_bentoml` for users. ```python from bentoml.deployment.clipper import deploy_bentoml deployed_model_name, deployed_model_version = deploy_bentoml( clipper_connection, bentoml_service_archive_path, api_name, input_type ) # After model is deployed to clipper cluster, can register application and link up application and deployed model to be an endpoint in clipper cluster ```

parano reviewed Jun 3, 2019

View reviewed changes

bentoml/archive/py_module_utils.py Outdated Show resolved Hide resolved

parano reviewed Jun 4, 2019

View reviewed changes

yubozhao force-pushed the clipper-support branch from becb5b1 to c5bd9d8 Compare June 6, 2019 21:36

yubozhao force-pushed the clipper-support branch from b269c79 to fb95c00 Compare July 10, 2019 22:54

yubozhao added 12 commits July 10, 2019 16:13

WIP clipper deployment

e85e183

WIP

87035d2

format styles

19e3113

fix linter issues

40b063c

WIP

ac2bee6

WIP

fb23d2d

remove deployment class for clipper

d8c29ff

WIP

a9b774a

add labels param for deploy clipper, update handler for clipper deplo…

771388c

…yment

WIP clipper example

8b10246

fix line too long

5902577

touchup styling

f71febe

yubozhao force-pushed the clipper-support branch from fb95c00 to f12c5a6 Compare July 11, 2019 18:21

Update templating file for clipper deployment

3ee4861

yubozhao force-pushed the clipper-support branch from f12c5a6 to 3ee4861 Compare July 11, 2019 22:45

update clipper example

fa1e612

yubozhao added 4 commits July 11, 2019 18:29

Add additional data type support for clipper

7503b49

setup input_type as parameter for deploy_bentoml

c2a386a

Add example notebook for deploy to clipper

9a5a5e7

fix styling

2932697

yubozhao commented Jul 15, 2019

View reviewed changes

yubozhao added 2 commits July 15, 2019 14:44

update example for clipper

a0c61d4

nit, small styling on markdown

400e3a5

yubozhao marked this pull request as ready for review July 15, 2019 22:01

yubozhao added 2 commits July 15, 2019 15:23

add deploy clipper to documentation

dc50ec3

nit from pep8

c019b97

parano reviewed Jul 16, 2019

View reviewed changes

update base on suggestions

45720a8

parano reviewed Jul 16, 2019

View reviewed changes

bentoml/handlers/dataframe_handler.py Outdated Show resolved Hide resolved

parano reviewed Jul 16, 2019

View reviewed changes

yubozhao added 4 commits July 16, 2019 15:06

update

458accb

use logger to display docker info instead of sys.stdout

92ba6e9

use correct log module

f05436a

Merge branch 'master' into clipper-support

dea4ea7

yubozhao merged commit 0fc85b5 into bentoml:master Jul 16, 2019

yubozhao deleted the clipper-support branch July 16, 2019 23:00

		return self.application_name


		def deploy_bentoml(clipper_conn, archive_path, api_name):

Comments

Conversation

yubozhao commented Jun 3, 2019

What changes were proposed in this pull request?

Does this close any currently open issues?

How was this patch tested?

Uh oh!

pep8speaks commented Jun 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2019-07-16 22:50:54 UTC

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jul 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yubozhao Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pep8speaks commented Jun 3, 2019 •

edited

Loading

codecov-io commented Jul 11, 2019 •

edited

Loading

yubozhao Jul 15, 2019 •

edited

Loading