Repository: bentoml/BentoML
Pull request #3794 (2023-04-26 07:48:38)

fix: allow import service defined under a Python package
## What does this PR address?

Currently when the service import string is pointing to a top-level Python package (a directory under working_dir containing a `__init__.py` file), e.g. the demo project here: https://github.com/bentoml/CLIP-API-service, BentoML fails to recognize it as a module name and try to treat it as a python file path.

This PR fixes this corner case and adds related tests for the service loading code.


<!--
Thanks for sending a pull request!

Congrats for making it this far! Here's a 🍱 for you. There are still a few steps ahead.

Please make sure to read the contribution guidelines, then fill out the blanks below before requesting a code review.

Name your Pull Request with one of the following prefixes, e.g. "feat: add support for PyTorch", to indicate the type of changes proposed. This is based on the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/#summary).
  - feat: (new feature for the user, not a new feature for build script)
  - fix: (bug fix for the user, not a fix to a build script)
  - docs: (changes to the documentation)
  - style: (formatting, missing semicolons, etc; no production code change)
  - refactor: (refactoring production code, eg. renaming a variable)
  - perf: (code changes that improve performance)
  - test: (adding missing tests, refactoring tests; no production code change)
  - chore: (updating grunt tasks etc; no production code change)
  - build: (changes that affect the build system or external dependencies)
  - ci: (changes to configuration files and scripts)
  - revert: (reverts a previous commit)

Describe your changes in detail. Attach screenshots here if appropriate.

Once you're done with this, someone from BentoML team or community member will help review your PR (see "Who can help review?" section for potential reviewers.). If no one has reviewed your PR after a week have passed, don't hesitate to post a new comment and ping @-the same person. Notifications sometimes get lost 🥲.
-->

<!-- Remove if not applicable -->

Fixes #(issue)

## Before submitting:

<!--- 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] Does the Pull Request follow [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/#summary) naming? Here are [GitHub's
      guide](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request) on how to create a pull request.
- [x] Does the code follow BentoML's code style, both `make format` and `make lint` script have passed ([instructions](https://github.com/bentoml/BentoML/blob/main/DEVELOPMENT.md#style-check-auto-formatting-type-checking))?
- [x] Did you read through [contribution guidelines](https://github.com/bentoml/BentoML/blob/main/CONTRIBUTING.md#ways-to-contribute) and follow [development guidelines](https://github.com/bentoml/BentoML/blob/main/DEVELOPMENT.md#start-developing)?
- [ ] Did your changes require updates to the documentation? Have you updated
      those accordingly? Here are [documentation guidelines](https://github.com/bentoml/BentoML/tree/main/docs) and [tips on writting docs](https://github.com/bentoml/BentoML/tree/main/docs#writing-documentation).
- [x] Did you write tests to cover your changes?


aarnphm: @parano should we also support `src` layout project as well?
parano: > @parano should we also support `src` layout project as well?

I think we can already support that, simply pass in `src` directory as the `--working-dir` for build and serve etc

Patch:
diff --git a/src/bentoml/_internal/service/loader.py b/src/bentoml/_internal/service/loader.py
index 05f730d6..681bc2eb 100644
--- a/src/bentoml/_internal/service/loader.py
+++ b/src/bentoml/_internal/service/loader.py
@@ -102,7 +102,7 @@ def import_service(
                 '"<module>:<attribute>" or "<module>'
             )
 
-        if os.path.exists(import_path):
+        if os.path.isfile(import_path):
             import_path = os.path.realpath(import_path)
             # Importing from a module file path:
             if not import_path.startswith(working_dir):


Test patch:
diff --git a/tests/unit/_internal/service_loader/bento_service_in_module/__init__.py b/tests/unit/_internal/service_loader/bento_service_in_module/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/tests/unit/_internal/service_loader/bento_service_in_module/multi_service.py b/tests/unit/_internal/service_loader/bento_service_in_module/multi_service.py
new file mode 100644
index 00000000..b9547afb
--- /dev/null
+++ b/tests/unit/_internal/service_loader/bento_service_in_module/multi_service.py
@@ -0,0 +1,4 @@
+import bentoml
+
+svc_i = bentoml.Service("test-bento-service-in-module-i")
+svc_ii = bentoml.Service("test-bento-service-in-module-ii")
diff --git a/tests/unit/_internal/service_loader/bento_service_in_module/single_service.py b/tests/unit/_internal/service_loader/bento_service_in_module/single_service.py
new file mode 100644
index 00000000..81ed9e27
--- /dev/null
+++ b/tests/unit/_internal/service_loader/bento_service_in_module/single_service.py
@@ -0,0 +1,3 @@
+import bentoml
+
+svc = bentoml.Service("test-bento-service-in-module-single")
diff --git a/tests/unit/_internal/service_loader/multi_service_in_package/__init__.py b/tests/unit/_internal/service_loader/multi_service_in_package/__init__.py
new file mode 100644
index 00000000..79cabfab
--- /dev/null
+++ b/tests/unit/_internal/service_loader/multi_service_in_package/__init__.py
@@ -0,0 +1,4 @@
+import bentoml
+
+svc_i = bentoml.Service("test-multi-service-in-package-i")
+svc_ii = bentoml.Service("test-multi-service-in-package-ii")
diff --git a/tests/unit/_internal/service_loader/single_service_in_package/__init__.py b/tests/unit/_internal/service_loader/single_service_in_package/__init__.py
new file mode 100644
index 00000000..4ca53400
--- /dev/null
+++ b/tests/unit/_internal/service_loader/single_service_in_package/__init__.py
@@ -0,0 +1,3 @@
+import bentoml
+
+svc = bentoml.Service("test-bento-service-in-package")
diff --git a/tests/unit/_internal/service_loader/test_import_service.py b/tests/unit/_internal/service_loader/test_import_service.py
new file mode 100644
index 00000000..b7ed6a8f
--- /dev/null
+++ b/tests/unit/_internal/service_loader/test_import_service.py
@@ -0,0 +1,59 @@
+import pytest
+
+from bentoml.exceptions import ImportServiceError
+from bentoml._internal.service.loader import import_service
+
+
+@pytest.mark.usefixtures("change_test_dir")
+def test_load_by_service_file():
+    svc = import_service("bento_service_in_module/single_service.py")
+    assert svc.name == "test-bento-service-in-module-single"
+
+    svc = import_service("./bento_service_in_module/single_service.py:svc")
+    assert svc.name == "test-bento-service-in-module-single"
+
+    svc = import_service("./single_service_in_package/__init__.py")
+    assert svc.name == "test-bento-service-in-package"
+
+    svc = import_service("./single_service_in_package/__init__.py:svc")
+    assert svc.name == "test-bento-service-in-package"
+
+
+@pytest.mark.usefixtures("change_test_dir")
+def test_load_by_module_name():
+    svc = import_service("bento_service_in_module.single_service")
+    assert svc.name == "test-bento-service-in-module-single"
+
+    svc = import_service("bento_service_in_module.single_service:svc")
+    assert svc.name == "test-bento-service-in-module-single"
+
+    svc = import_service("single_service_in_package")
+    assert svc.name == "test-bento-service-in-package"
+
+    svc = import_service("single_service_in_package:svc")
+    assert svc.name == "test-bento-service-in-package"
+
+
+@pytest.mark.usefixtures("change_test_dir")
+def test_load_multi_service_module():
+    svc = import_service("bento_service_in_module.multi_service:svc_i")
+    assert svc.name == "test-bento-service-in-module-i"
+
+    svc = import_service("bento_service_in_module.multi_service:svc_ii")
+    assert svc.name == "test-bento-service-in-module-ii"
+
+    with pytest.raises(
+        ImportServiceError, match="Multiple Service instances found in module"
+    ):
+        import_service("bento_service_in_module.multi_service")
+
+    svc = import_service("multi_service_in_package:svc_i")
+    assert svc.name == "test-multi-service-in-package-i"
+
+    svc = import_service("multi_service_in_package:svc_ii")
+    assert svc.name == "test-multi-service-in-package-ii"
+
+    with pytest.raises(
+        ImportServiceError, match="Multiple Service instances found in module"
+    ):
+        import_service("multi_service_in_package")