bentoml / BentoML

The easiest way to serve AI apps and models - Build Model Inference APIs, Job queues, LLM apps, Multi-model pipelines, and more!
https://bentoml.com
Apache License 2.0
7.13k stars 791 forks source link

bug: Docker env option does not support paths #3301

Closed ChristianMarzahl closed 1 year ago

ChristianMarzahl commented 1 year ago

Describe the bug

Setting environment variables with path characters result in an error, this prohibits a large range of valid environment variables from being set without a custom build. #3268

docker:
    env:
        - BENTOML_CONFIG=./bento_config.yaml

results in:

Eception Group Traceback (most recent call last):
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/bin/bentoml", line 8, in <module>
  |     sys.exit(cli())
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/click/core.py", line 1128, in __call__
  |     return self.main(*args, **kwargs)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/click/core.py", line 1053, in main
  |     rv = self.invoke(ctx)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/click/core.py", line 1659, in invoke
  |     return _process_result(sub_ctx.command.invoke(sub_ctx))
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/click/core.py", line 1395, in invoke
  |     return ctx.invoke(self.callback, **ctx.params)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/click/core.py", line 754, in invoke
  |     return __callback(*args, **kwargs)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/bentoml_cli/utils.py", line 164, in wrapper
  |     return func(*args, **kwargs)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/bentoml_cli/utils.py", line 138, in wrapper
  |     return_value = func(*args, **kwargs)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/bentoml_cli/utils.py", line 99, in wrapper
  |     return func(*args, **kwargs)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/bentoml_cli/bentos.py", line 280, in build
  |     build_bentofile(bentofile, build_ctx=build_ctx, version=version)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/simple_di/__init__.py", line 139, in _
  |     return func(*_inject_args(bind.args), **_inject_kwargs(bind.kwargs))
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/bentoml/bentos.py", line 368, in build_bentofile
  |     build_config = BentoBuildConfig.from_yaml(f)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/bentoml/_internal/bento/build_config.py", line 723, in from_yaml
  |     return bentoml_cattr.structure(yaml_content, cls)
  |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/cattrs/converters.py", line 281, in structure
  |     return self._structure_func.dispatch(cl)(obj, cl)
  |   File "<cattrs generated structure bentoml._internal.bento.build_config.BentoBuildConfig>", line 54, in structure_BentoBuildConfig
  |     if errors: raise __c_cve('While structuring BentoBuildConfig', errors, __cl)
  | cattrs.errors.ClassValidationError: While structuring BentoBuildConfig (1 sub-exception)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "<cattrs generated structure bentoml._internal.bento.build_config.DockerOptions-2>", line 57, in structure_DockerOptions
    |     return __cl(
    |   File "<attrs generated init bentoml._internal.bento.build_config.DockerOptions>", line 5, in __init__
    |     _setattr(self, 'env', __attr_converter_env(env))
    |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/bentoml/_internal/bento/build_config.py", line 111, in _convert_env
    |     raise BentoMLException(
    | bentoml.exceptions.BentoMLException: All value in `env` list must follow format ENV=VALUE
    |
    | During handling of the above exception, another exception occurred:
    |
    | Exception Group Traceback (most recent call last):
    |   File "<cattrs generated structure bentoml._internal.bento.build_config.BentoBuildConfig>", line 35, in structure_BentoBuildConfig
    |     res['docker'] = __c_structure_docker(o['docker'], __c_type_docker)
    |   File "<cattrs generated structure bentoml._internal.bento.build_config.DockerOptions-2>", line 60, in structure_DockerOptions
    |     except Exception as exc: raise __c_cve('While structuring DockerOptions', [exc], __cl)
    | cattrs.errors.ClassValidationError: While structuring DockerOptions (1 sub-exception)
    +-+---------------- 1 ----------------
      | Traceback (most recent call last):
      |   File "<cattrs generated structure bentoml._internal.bento.build_config.DockerOptions-2>", line 57, in structure_DockerOptions
      |     return __cl(
      |   File "<attrs generated init bentoml._internal.bento.build_config.DockerOptions>", line 5, in __init__
      |     _setattr(self, 'env', __attr_converter_env(env))
      |   File "/home/cmarzahl/anaconda3/envs/icevision_116/lib/python3.8/site-packages/bentoml/_internal/bento/build_config.py", line 111, in _convert_env
      |     raise BentoMLException(
      | bentoml.exceptions.BentoMLException: All value in `env` list must follow format ENV=VALUE

To reproduce

docker:
    env:
        - BENTOML_CONFIG=./bento_config.yaml

Expected behavior

Behaviour like a standard script export:

export BENTOML_CONFIG=/mnt/c/ProgProjekte/Python/project/bento_config.yaml

Environment

bentoml: >1.0.5 python: 3.8.13 os: UBUNTU 20.04

aarnphm commented 1 year ago

Thanks for reporting this bug. We will need to handle path-like objects separately for this.

I notice that you are using a bento_config.yaml. I suggest you not include this inside the bento, rather mounting this into the container as BENTOML_CONFIG content is subject to change in the future with newer versions.

docker run -e BENTOML_CONFIG=/home/bentoml/bentoml_config.yaml -v /path/to/your_config_file_locally.yaml:/home/bentoml/bentoml_config.yaml ...
ChristianMarzahl commented 1 year ago

Dear @aarnphm,

Thank you very much for your quick response and pointing out this solution.

KimSoungRyoul commented 1 year ago

@aarnphm

do you have a plan support DEFAULT_BENTO_CONFIG?

like a

# dockerfile
DEFAULT_BENTO_CONFIG = "configuration.yaml"

if exists("$BENTO_PATH/configuration.yaml"): # check file exists
   export BENTO_CONFIG="$BENTO_PATH/configuration.yaml"
aarnphm commented 1 year ago

This is an anti-pattern to include the bento_config into the bento, and we won't do anything special check for the config namespace yet.

BENTOML_CONFIG should always be mounted into the container rather than being included in the container. The reason why I have already covered above.

So the answer is No 😃