[Refactor] rename API Handlers to Inputs #784

bojiang · 2020-06-11T15:57:54Z

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Documentation
Test, CI, or build
None

Components (if applicable)

BentoService (model packaging, dependency management, handler definition)
Model Artifact (model serialization, multi-framework support)
Model Server (mico-batching, logging, metrics, tracing, benchmark, OpenAPI)
YataiService (model management, deployment automation)
Documentation

Checklist:

My code follows the bentoml code style, both ./dev/format.sh and
./dev/lint.sh script have passed
(instructions).
My change requires a change to the documentation.
I have updated the documentation accordingly.
My change requires a change in bentoml/gallery example notebooks
I have sent a pull request to bentoml/gallery to make that change

TODOs:

Renaming all handler in docs
Add deprecated warnings for bentoml.handlers
Check config version while loading

codecov · 2020-06-12T03:24:45Z

Codecov Report

Merging #784 into master will increase coverage by 0.21%.
The diff coverage is 64.08%.

@@            Coverage Diff             @@
##           master     #784      +/-   ##
==========================================
+ Coverage   55.53%   55.75%   +0.21%     
==========================================
  Files         108      114       +6     
  Lines        8144     8346     +202     
==========================================
+ Hits         4523     4653     +130     
- Misses       3621     3693      +72

Impacted Files	Coverage Δ
bentoml/adapters/pytorch_tensor_input.py	`0.00% <0.00%> (ø)`
bentoml/artifact/fastai_model_artifact.py	`45.00% <ø> (ø)`
bentoml/artifact/fasttext_model_artifact.py	`43.33% <ø> (ø)`
bentoml/artifact/h2o_model_artifact.py	`41.17% <ø> (ø)`
bentoml/artifact/keras_model_artifact.py	`21.09% <ø> (ø)`
bentoml/artifact/lightgbm_model_artifact.py	`40.00% <ø> (ø)`
bentoml/artifact/onnx_model_artifact.py	`85.13% <ø> (ø)`
bentoml/artifact/pytorch_model_artifact.py	`85.36% <ø> (ø)`
bentoml/artifact/sklearn_model_artifact.py	`70.73% <ø> (ø)`
bentoml/artifact/spacy_model_artifact.py	`40.54% <ø> (ø)`
... and 39 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 bb9a327...690bec8. Read the comment docs.

parano · 2020-06-12T03:27:48Z

bentoml/clipper/__init__.py

        raise BentoMLException(
            "Only BentoService APIs using ClipperHandler can be deployed to Clipper"
        )

-    input_type = HANDLER_TYPE_TO_INPUT_TYPE[api_metadata.handler_type]
+    input_type = HANDLER_TYPE_TO_INPUT_TYPE[api_metadata.input_type]


maybe change this to something like to_clipper_input_type(api_metadata.input_type and CLIPPER_INPUTS?

parano · 2020-06-12T03:54:45Z

bentoml/cli/bento.py

@@ -47,7 +47,7 @@ def _print_bento_table(bentos, wide=False):
            for artifact in bento.bento_service_metadata.artifacts
        ]
        apis = [
-            f'{api.name}<{api.handler_type}>'
+            f'{api.name}<{api.input_type}:{api.output_type}>'


We may also need to update the Web UI for this, @yubozhao could you help identify the files that need similar change?

Yep.
We would need to update the table view here:

BentoML/bentoml/yatai/web/client/src/components/BentoServiceTable.tsx

Line 11 in 8d49f11

return apis.map((api) => `${api.name}<${api.handler_type}>`).join("\n");

and the API table:

BentoML/bentoml/yatai/web/client/src/components/BentoServiceDetail/ApisTable.tsx

Line 57 in 8d49f11

const ApisTable: React.FC<{ apis: Array<IApiProps> }> = ({ apis }) => {

parano · 2020-06-12T04:07:30Z

protos/repository.proto

    string docs = 3;
+    string output_type = 6;

    // Configs for customizing BentoHandler


Also update the inline docs here?

parano · 2020-06-12T04:09:07Z

tests/bento_service_examples/example_bento_service.py

    def predict_dataframe(self, df):
        """predict_dataframe expects dataframe as input
        """
        return self.artifacts.model.predict_dataframe(df)

-    @bentoml.api(DataframeHandler, input_dtypes={"col1": "int"})
+    @bentoml.api(DataframeInput, input_dtypes={"col1": "int"})


parano · 2020-06-12T04:32:37Z

bentoml/adapters/__init__.py

+LegacyImageInput = LegacyImageHandler
+FastaiImageInput = FastaiImageHandler
+
+ClipperBytesInput = ClipperBytesHandler


Maybe make it the other way around?

E.g. rename ImageHandler class to ImageInput, and add ImageHandler = ImageInput here?

It will be easier to deprecate those alias later on, and easier to new contributors to understand the codebase.

Let me know if you plan to do that in a separate PR

Yeah, working in progress.

Actually, would be nice to add a deprecation warning when the user is using the "handler" APIs. Would something like this work?

class ImageHandler(ImageInput): """ backwards compatible alias ..... """ def __init__(...): super... logger.warning("will be deprecated .... use ImageInput")

bojiang · 2020-06-12T06:45:42Z

@parano done. Let's leave deprecation warnings in next PR

parano · 2020-06-12T10:15:09Z

@yubozhao could you help test out Lambda and SageMaker deployment with this change?

* [Refactor] rename handlers to input adapters * [Refactor] rename BentoHandler * [FIX] renaming handler_type in frontend * Rename ClipperHandlers and DataframeHandler * Rename rest Handlers

bojiang force-pushed the dev3 branch 2 times, most recently from 65dd92a to 477d891 Compare June 12, 2020 03:15

parano reviewed Jun 12, 2020

View reviewed changes

bojiang force-pushed the dev3 branch from c33f60d to ff67b05 Compare June 12, 2020 06:35

bojiang added 5 commits June 12, 2020 14:53

[Refactor] rename handlers to input adapters

c3d7aa5

[Refactor] rename BentoHandler

b63d875

[FIX] renaming handler_type in frontend

d97114f

Rename ClipperHandlers and DataframeHandler

6c83d5a

Rename rest Handlers

690bec8

bojiang force-pushed the dev3 branch from a72c8d2 to 690bec8 Compare June 12, 2020 06:54

parano changed the title ~~[Refactor] rename handlers to inputs~~ [Refactor] rename API Handlers to Inputs Jun 12, 2020

parano merged commit a134e3a into bentoml:master Jun 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Refactor] rename API Handlers to Inputs #784

[Refactor] rename API Handlers to Inputs #784

bojiang commented Jun 11, 2020 •

edited

codecov bot commented Jun 12, 2020 •

edited

parano Jun 12, 2020

parano Jun 12, 2020

yubozhao Jun 12, 2020

parano Jun 12, 2020

parano Jun 12, 2020

parano Jun 12, 2020 •

edited

bojiang Jun 12, 2020

bojiang Jun 12, 2020

parano Jun 12, 2020

bojiang commented Jun 12, 2020 •

edited

parano commented Jun 12, 2020

[Refactor] rename API Handlers to Inputs #784

[Refactor] rename API Handlers to Inputs #784

Conversation

bojiang commented Jun 11, 2020 • edited

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Components (if applicable)

Checklist:

codecov bot commented Jun 12, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parano Jun 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bojiang commented Jun 12, 2020 • edited

parano commented Jun 12, 2020

bojiang commented Jun 11, 2020 •

edited

codecov bot commented Jun 12, 2020 •

edited

parano Jun 12, 2020 •

edited

bojiang commented Jun 12, 2020 •

edited