Customize route of API endpoint #1334

bojiang · 2020-12-17T02:10:05Z

Description

Motivation and Context

How Has This Been Tested?

Provided integration tests.
It also passed the backward compatibility tests. See #1370

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature and improvements (non-breaking change which adds/improves functionality)
Bug fix (non-breaking change which fixes an issue)
Code Refactoring (internal change which is not user facing)
Documentation
Test, CI, or build

Component(s) if applicable

BentoService (service definition, dependency management, API input/output adapters)
Model Artifact (model serialization, multi-framework support)
Model Server (mico-batching, dockerisation, logging, OpenAPI, instruments)
YataiService gRPC server (model registry, cloud deployment automation)
YataiService web server (nodejs HTTP server and web UI)
Internal (BentoML's own configuration, logging, utility, exception handling)
BentoML CLI

Checklist:

My code follows the bentoml code style, both ./dev/format.sh and
./dev/lint.sh script have passed
(instructions).
My change reduces project test coverage and requires unit tests to be added
I have added unit tests covering my code change
My change requires a change to the documentation
I have updated the documentation accordingly

codecov · 2020-12-17T02:12:14Z

Codecov Report

Merging #1334 (db741ff) into master (bad87f1) will increase coverage by 0.09%.
The diff coverage is 58.82%.

@@            Coverage Diff             @@
##           master    #1334      +/-   ##
==========================================
+ Coverage   67.72%   67.82%   +0.09%     
==========================================
  Files         150      150              
  Lines       10017    10029      +12     
==========================================
+ Hits         6784     6802      +18     
+ Misses       3233     3227       -6

Impacted Files	Coverage Δ
bentoml/yatai/proto/repository_pb2.py	`100.00% <ø> (ø)`
bentoml/marshal/marshal.py	`25.82% <31.25%> (+0.16%)`	⬆️
bentoml/saved_bundle/config.py	`88.32% <75.00%> (-0.49%)`	⬇️
bentoml/service/__init__.py	`75.49% <80.00%> (+0.07%)`	⬆️
bentoml/server/api_server.py	`71.42% <100.00%> (ø)`
bentoml/server/open_api.py	`100.00% <100.00%> (ø)`
bentoml/service/inference_api.py	`85.71% <100.00%> (+0.09%)`	⬆️
bentoml/saved_bundle/loader.py	`62.68% <0.00%> (+1.49%)`	⬆️
bentoml/adapters/default_output.py	`86.36% <0.00%> (+4.54%)`	⬆️
bentoml/adapters/json_output.py	`80.95% <0.00%> (+9.52%)`	⬆️

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 bad87f1...db741ff. Read the comment docs.

yubozhao · 2020-12-22T21:35:40Z

bentoml/service/__init__.py

@@ -66,11 +66,23 @@ def validate_inference_api_name(api_name: str):
        )


+def validate_inference_api_route(route: str):
+    if re.findall(r"[?#]+|^(//)|^:", route):


Let's add documentation for the regex.

Should we also invalidate route input like abc/123/ with trailing '/'?

The route with a trailing "/" also works.

yubozhao · 2020-12-22T21:55:44Z

tests/integration/api_server/test_adapter.py

@@ -106,6 +106,22 @@ async def test_api_server_json(host):
    await asyncio.gather(*tasks)


+@pytest.mark.asyncio
+async def test_api_server_json_with_customized_route(host):


I am getting errors when I run pytest on api_server. 429 'Too Many Requests'

It's normal. It happens when your test server is busy.

parano · 2020-12-30T06:25:36Z

bentoml/saved_bundle/config.py

@@ -206,6 +208,12 @@ def get_bento_service_metadata_pb(self):
                    else:
                        api_metadata.mb_max_batch_size = DEFAULT_MAX_BATCH_SIZE

+                    # Supports customize route from 0.10.2


Remove this comment? the next release will be 0.11.0

parano · 2020-12-30T06:26:40Z

bentoml/saved_bundle/config.py

+                    # Supports customize route from 0.10.2
+                    if 'route' in api_config:
+                        api_metadata.route = api_config["route"]
+                    else:


suggest adding a comment here Use API name as the URL route when route config is missing, this is for backward compatibility for BentoML version <= 0.10.1

parano · 2020-12-30T06:33:54Z

bentoml/service/inference_api.py

    ):
        """
        :param service: ref to service containing this API
        :param name: API name
+        :param route: API path (by default the same as `name`)


Suggest update:

:param route: Specify HTTP URL route of this inference API. By default, `api.name` is used as the route. This parameter can be used for customizing the URL route, e.g. `route="/api/v2/model_a/predict"`

And should we move the docstring order to match the order of the actual parameter list in this method?

parano · 2020-12-30T06:38:05Z

bentoml/service/__init__.py

@@ -85,6 +100,8 @@ def api_decorator(
    :param output: OutputAdapter instance of the inference API
    :param api_name: API name, default to the user-defined callback function's function
        name
+    :param route: to customize the route of API


same as the suggestion above

bojiang · 2021-02-10T08:03:53Z

@parano ready

bojiang · 2021-02-24T12:41:22Z

conflict resolved

parano · 2021-02-25T00:50:13Z

bentoml/service/__init__.py

@@ -86,6 +101,10 @@ def api_decorator(
    :param output: OutputAdapter instance of the inference API
    :param api_name: API name, default to the user-defined callback function's function
        name
+    :param route: Specify HTTP URL route of this inference API. By default,


one minor issue here is that we want to enforce that all inference APIs in a BentoService must have distinct route. previously it was not an issue because Python enforces users to choose a different function name implicitly, but now we may want to do a validation somewhere

maybe something we could do in _config_inference_apis below?

parano · 2021-02-25T00:53:21Z

tests/conftest.py

@@ -64,6 +67,36 @@ async def assert_request(
        'columns',
    }

+    def _since_version(ver: str):


* add test for customized route * customize route * fix * add route test * add url character test * add illegal url characters test * add unittest for customize route * review * gen proto * fix * fix 2 * [CI] add `since_bentoml_version` for itests * fix * fix * fix3 * without docker Co-authored-by: ruhan <[email protected]>

bojiang force-pushed the dev6 branch 2 times, most recently from 8788ebe to 6f7a4b5 Compare December 18, 2020 09:14

yubozhao mentioned this pull request Dec 21, 2020

[Feature] Customizable request schema #1347

Merged

18 tasks

bojiang force-pushed the dev6 branch from 58652dd to d0e3e16 Compare December 22, 2020 03:44

bojiang marked this pull request as ready for review December 22, 2020 03:53

bojiang requested review from parano and yubozhao and removed request for parano December 22, 2020 03:54

yubozhao reviewed Dec 22, 2020

View reviewed changes

bojiang force-pushed the dev6 branch from da70ac3 to 887e123 Compare December 24, 2020 07:20

parano reviewed Dec 30, 2020

View reviewed changes

bojiang force-pushed the dev6 branch 2 times, most recently from 2da2149 to 125ff8c Compare January 6, 2021 08:39

bojiang force-pushed the dev6 branch 3 times, most recently from 3090ba0 to 22f37bc Compare February 10, 2021 02:38

bojiang and others added 7 commits February 24, 2021 18:52

add test for customized route

1365dae

customize route

54b5dd9

fix

6a62467

add route test

2aa9e1b

add url character test

16c4f84

add illegal url characters test

15f7913

add unittest for customize route

31f1150

bojiang added 9 commits February 24, 2021 18:54

review

face3b8

gen proto

206b30b

fix

5961df3

fix 2

36e5589

[CI] add since_bentoml_version for itests

8938e1f

fix

7dbf768

fix

252cecc

fix3

142af56

without docker

db741ff

bojiang force-pushed the dev6 branch from 11474e2 to db741ff Compare February 24, 2021 10:55

parano reviewed Feb 25, 2021

View reviewed changes

tests/conftest.py

@@ -64,6 +67,36 @@ async def assert_request(

'columns',

}

def _since_version(ver: str):

Copy link

Member

parano Feb 25, 2021

Choose a reason for hiding this comment

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

👍

parano merged commit 557f2fa into bentoml:master Feb 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Customize route of API endpoint #1334

Customize route of API endpoint #1334

bojiang commented Dec 17, 2020 •

edited

Loading

codecov bot commented Dec 17, 2020 •

edited

Loading

yubozhao Dec 22, 2020

yubozhao Dec 22, 2020

bojiang Feb 10, 2021

yubozhao Dec 22, 2020

bojiang Dec 25, 2020

parano Dec 30, 2020

bojiang Feb 8, 2021

parano Dec 30, 2020

bojiang Feb 8, 2021

parano Dec 30, 2020 •

edited

Loading

parano Dec 30, 2020

bojiang Feb 8, 2021

parano Dec 30, 2020

bojiang Feb 8, 2021

bojiang commented Feb 10, 2021

bojiang commented Feb 24, 2021

parano Feb 25, 2021

parano Feb 25, 2021

parano Feb 25, 2021

Customize route of API endpoint #1334

Customize route of API endpoint #1334

Conversation

bojiang commented Dec 17, 2020 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Component(s) if applicable

Checklist:

codecov bot commented Dec 17, 2020 • edited Loading

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

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 Dec 30, 2020 • edited Loading

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

bojiang commented Feb 10, 2021

bojiang commented Feb 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bojiang commented Dec 17, 2020 •

edited

Loading

codecov bot commented Dec 17, 2020 •

edited

Loading

parano Dec 30, 2020 •

edited

Loading