Repository: bentoml/BentoML
Pull request: Custom Web UI support for API Server
<!--- Thanks for sending a pull request! Please make sure to read the contribution guidelines, then fill out the blanks below before requesting a code review. -->

## Description
<!--- Describe your changes in detail -->
<!--- Attach screenshots here if appropriate. -->
Closes #739 

Requires a directory to be passed containing `index.html` and `static` dir.
**Note**: Any files not inside `static` will not be accessible. 
Example deployment: https://bentoml-her0ku-mtu5mtkwnjq0nao.herokuapp.com/

The example deployment is still WIP and has no loading indicator/error handling.
Please recommend changes for the demo deployment so the same can be used for documentation.

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
<!--- If it is based on a conversation in slack channel, pls quote related messages here -->
Implements #739 

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
Tested locally and deployed on Heroku

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] 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
<!--- [ ] Others: describe the type of change if it does not fit to categories listed -->

## Component(s) if applicable
- [x] BentoService (service definition, dependency management, API input/output adapters)
- [ ] Model Artifact (model serialization, multi-framework support)
- [x] 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:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
<!--- If you plan to update documentation or tests in follow-up, please note -->
- [x] My code follows the bentoml code style, both `./dev/format.sh` and
  `./dev/lint.sh` script have passed
  ([instructions](https://github.com/bentoml/BentoML/blob/master/DEVELOPMENT.md#style-check-and-auto-formatting-your-code)).
- [x] My change reduces project test coverage and requires unit tests to be added
- [ ] I have added unit tests covering my code change
- [x] My change requires a change to the documentation
- [ ] I have updated the documentation accordingly


Problem statement:
Custom service web UI to replace default swagger UI
* Currently when starting a BentoML API model server, it starts a simple web UI in addition to the API endpoints. This Web UI is generated with swagger API schema, it can be used for data scientists to test the API endpoint from a browser. But not ideal for a non-technical user to use.

* This proposal is to allow BentoML user to provide a tarfile when building a BentoService saved bundle, which contains static content being hosted web page code, including a `index.html` and optionally other JS code.

* User javascript code should be able to send prediction request to the API model server for prediction

* This makes it possible for users to customize the web UI of their ML application for end-user to consume directly. It can be very powerful for building internal tools.


API design draft:
```python
@env(..)
@artifacts([...])
@web_static_files("./web_ui/*")
class MyPredictionService(BentoService)
     
         @api(...)
         def predict(...)
              pass
```


BentoML should provide a few template frontend projects for a user to get started with building a web UI easily. e.g. a basic HTML form submitting input data, a file upload window that can be used to drag-n-drop a image or audio file, a d3 visualization that visualized the prediction result, etc

An advanced version of this should allow user to declared UI elements right from their Python code. similar to how R Shinny works.

Hints from issue discussion:
Hey @parano, I would like to work on this if no one else is already working on it
Thanks, @Korusuke, let me know if you have any questions about it

Commit message: Fix _path

Patch to review:
diff --git a/bentoml/__init__.py b/bentoml/__init__.py
index 68eec2f58bf..b08631ff8ef 100644
--- a/bentoml/__init__.py
+++ b/bentoml/__init__.py
@@ -29,6 +29,7 @@
     BentoService,
     api_decorator as api,
     env_decorator as env,
+    webui_decorator as webui,
     artifacts_decorator as artifacts,
     ver_decorator as ver,
     save,
@@ -41,6 +42,7 @@
     "artifacts",
     "config",
     "env",
+    "webui",
     "ver",
     "BentoService",
     "load",
diff --git a/bentoml/saved_bundle/bundler.py b/bentoml/saved_bundle/bundler.py
index df0d47d819c..7b9be66c6ba 100644
--- a/bentoml/saved_bundle/bundler.py
+++ b/bentoml/saved_bundle/bundler.py
@@ -142,6 +142,13 @@ def save_to_dir(bento_service, path, version=None, silent=False):
             )
         )
 
+    # copy custom webui if enabled
+    if hasattr(bento_service, '_static_files'):
+        src_webui_dir = os.path.join(os.getcwd(), bento_service._static_files)
+        dest_webui_dir = os.path.join(module_base_path, 'webui')
+        # os.mkdir(dest_webui_dir)
+        shutil.copytree(src_webui_dir, dest_webui_dir)
+
     # Copy docker-entrypoint.sh
     docker_entrypoint_sh_file_src = os.path.join(
         os.path.dirname(__file__), "docker-entrypoint.sh"
diff --git a/bentoml/saved_bundle/loader.py b/bentoml/saved_bundle/loader.py
index 779b38f7a48..734be28e72b 100644
--- a/bentoml/saved_bundle/loader.py
+++ b/bentoml/saved_bundle/loader.py
@@ -177,6 +177,7 @@ def load(bundle_path):
 
     svc_cls = load_bento_service_class(bundle_path)
     svc = svc_cls()
+    svc._path = bundle_path
 
     track_load_finish(svc)
     return svc
diff --git a/bentoml/server/bento_api_server.py b/bentoml/server/bento_api_server.py
index e530c15720e..4d14586d31c 100644
--- a/bentoml/server/bento_api_server.py
+++ b/bentoml/server/bento_api_server.py
@@ -20,7 +20,7 @@
 import logging
 from functools import partial
 
-from flask import Flask, jsonify, Response, request, make_response
+from flask import Flask, jsonify, Response, request, make_response, render_template
 from werkzeug.utils import secure_filename
 
 from bentoml import config
@@ -83,13 +83,23 @@ def __init__(self, bento_service, port=_DEFAULT_PORT, app_name=None):
 
         self.port = port
         self.bento_service = bento_service
-
-        self.app = Flask(
-            app_name,
-            static_folder=os.path.join(
-                os.path.dirname(os.path.abspath(__file__)), 'static'
-            ),
-        )
+        if hasattr(bento_service, '_static_files'):
+            self.app = Flask(
+                app_name,
+                static_folder=os.path.join(
+                    self.bento_service._path, app_name, 'webui/static'
+                ),
+                template_folder=os.path.join(
+                    self.bento_service._path, app_name, 'webui'
+                ),
+            )
+        else:
+            self.app = Flask(
+                app_name,
+                static_folder=os.path.join(
+                    os.path.dirname(os.path.abspath(__file__)), 'static'
+                ),
+            )
 
         for middleware in (InstrumentMiddleware,):
             self.app.wsgi_app = middleware(self.app.wsgi_app, self.bento_service)
@@ -106,16 +116,18 @@ def start(self):
         # Flask dev server enabled threaded by default, disable it.
         self.app.run(port=self.port, threaded=False)
 
-    @staticmethod
-    def index_view_func():
+    def index_view_func(self):
         """
         The index route for BentoML API server
         """
-        return Response(
-            response=INDEX_HTML.format(url='/docs.json'),
-            status=200,
-            mimetype="text/html",
-        )
+        if hasattr(self.bento_service, '_static_files'):
+            return render_template('index.html')
+        else:
+            return Response(
+                response=INDEX_HTML.format(url='/docs.json'),
+                status=200,
+                mimetype="text/html",
+            )
 
     @staticmethod
     def docs_view_func(bento_service):
diff --git a/bentoml/service.py b/bentoml/service.py
index 91acebde8d5..e50f3a39465 100644
--- a/bentoml/service.py
+++ b/bentoml/service.py
@@ -299,6 +299,24 @@ def decorator(func):
     return decorator
 
 
+def webui_decorator(static_files):
+    """Define web UI static files required to be bundled with a BentoService
+
+    Args:
+        static_files: path to directory containg index.html and static dir
+
+    >>>  @webui('./ui/')
+    >>>  class MyMLService(BentoService):
+    >>>     pass
+    """
+
+    def decorator(bento_service_cls):
+        bento_service_cls._static_files = static_files
+        return bento_service_cls
+
+    return decorator
+
+
 def artifacts_decorator(artifacts):
     """Define artifacts required to be bundled with a BentoService
 


Code review comments:
diff_hunk: @@ -106,16 +116,18 @@ def start(self):
         # Flask dev server enabled threaded by default, disable it.
         self.app.run(port=self.port, threaded=False)
 
-    @staticmethod
-    def index_view_func():
+    def index_view_func(self):
         """
         The index route for BentoML API server
         """
-        return Response(
-            response=INDEX_HTML.format(url='/docs.json'),
-            status=200,
-            mimetype="text/html",
-        )
+        if hasattr(self.bento_service, '_static_files'):
+            return render_template('index.html')
path: bentoml/server/bento_api_server.py
text: @author:
In case custom UI is enabled, do we want to also have `swagger` UI on some different endpoint?

@user1:
Yea, that'd be great for debugging, I think we can add a `/swagger.html` endpoint which always renders the swagger UI

@user1:
Does it actually need the `redner_template` method? the user can not really use the templating functionality right?  I'm more inclined to simply put all the static files under the `static` directory if they are just static files

@author:
Makes sense!

so now the dir structure will be:
```
webui_dir/
     |-- js/
     |--css/
     |--imgs/
     |--index.html
```
Here `webui_dir` is the supplied path by the user.

instead of `render_template` now it uses `send_from_directory`

@author:
> Yea, that'd be great for debugging, I think we can add a `/swagger.html` endpoint which always renders the swagger UI

In my opinion, `/docs` endpoint would be better for `swagger UI` since this is the same as what `fastapi` uses. And also if in future we change from `swagger` to something else, the path can remain the same. 

So `/docs` for the HTML page and `/docs.json` for openapi json.

What do you think?

@user1:
@author Yea I think `/docs` for the UI page and `/docs.json`for the json sounds great!



@user1:
What's the reason for `send_from_directory` instead of just the `static_folder` parameter?

@author:
> What's the reason for `send_from_directory` instead of just the `static_folder` parameter?

For serving `index.html` on `/` endpoint. if we don't use `send_from_directory` then only `/index.html` will work

@user1:
I believe it should serve all files under the static_folder directory not just index.html, you may also need to specify static_url_path: `app = Flask(__name__, static_folder='../web_client', static_url_path='/')`
and in your index view function do something like ```app.send_static_file('index.html')```

Adding static_url_path might break the swagger js/css files tho, you may need to adjust accordingly

@user1:
I think this way it is more friendly to the web frontend developer, for example, if the user creates the frontend project with something like https://github.com/facebook/create-react-app or use webpack to build their js project, they can simply supply the built `dist` directory for BentoML to serve as static files, and everything should just work.

@author:
> I believe it should serve all files under the static_folder directory not just index.html, you may also need to specify static_url_path: `app = Flask(__name__, static_folder='../web_client', static_url_path='/')`
> and in your index view function do something like `app.send_static_file('index.html')`

yeh, this is working currently!

> Adding static_url_path might break the swagger js/css files tho, you may need to adjust accordingly

Fixed that using send_from_directory and changing swagger static dir name.

> I think this way it is more friendly to the web frontend developer, for example, if the user creates the frontend project with something like https://github.com/facebook/create-react-app or use webpack to build their js project, they can simply supply the built `dist` directory for BentoML to serve as static files, and everything should just work.

Yep, I am trying to achieve this...currently the only issue is serving `any_path/index.html` for  `any_path/`, `any_path/index.html` and also `any_path/index`.

trying to find some better solution instead of just writing down all the cases.

@author:
One another doubt is - should we allow user-defined routes to override the system defined (predefined by bento)?

currently, if the user-defined function name is `index` then the server doesn't start, so should we allow to override that?
this may provide some extra flexibility but may even be a bit confusing at times.

@user1:
> currently, if the user-defined function name is index then the server doesn't start, so should we allow to override that?

`.. the server doesn't start ..`  is that because it has added two rules handling the same path?  I think it is probably not a good idea to allow user-defined `@user2` to override the system ones. Maybe we should validate the `@user2` name and make sure it is not in the reserved name list, such as `index`, `healthz`, `metrics` etc.  Currently there's only a `isidentifier` check, you can add this validation here: https://github.com/bentoml/BentoML/blob/v0.8.2/bentoml/service.py#L311



@author:
> 
> `.. the server doesn't start ..` is that because it has added two rules handling the same path?

Yep

Okey great!
should I include this in the same PR or create another issue/PR for this?

@user1:
@author a smaller PR that adds the validation would be nice, but it's ok to just include it in this PR as well, whichever is easier for you

diff_hunk: @@ -177,6 +177,7 @@ def load(bundle_path):
 
     svc_cls = load_bento_service_class(bundle_path)
     svc = svc_cls()
+    svc._path = bundle_path
path: bentoml/saved_bundle/loader.py
text: @author:
I am not sure about this, there may be a better way of getting the bundle path.

@user1:
you can use `svc._bento_service_bundle_path`, which is equivalent and used for loading artifacts

diff_hunk: @@ -142,6 +142,13 @@ def save_to_dir(bento_service, path, version=None, silent=False):
             )
         )
 
+    # copy custom webui if enabled
path: bentoml/saved_bundle/bundler.py
text: @user2:
My instinct is to add a property to the `bento_service`, e.g.

```
@user1
def static_files(self):
    """Returns static files if present.

    Raises:
        AttributeError if no static files
    """
    return self._static_files
```

but the maintainers can advise you more.

Motivation: underscore-prefixed attributes are "private", so it doesn't make sense to access them externally like this.

@author:
Done!