-
Notifications
You must be signed in to change notification settings - Fork 793
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: loading service from an absolute path #4415
Conversation
f8e8edf
to
efd34b7
Compare
src/_bentoml_impl/loader.py
Outdated
return service_identifier, pathlib.Path(working_dir or ".") | ||
if not module_path.is_absolute(): | ||
module_path = pathlib.Path(working_dir or ".").joinpath(module_path) | ||
return f"{module_path.stem}:{service_path}", module_path.parent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we return CWD as the bento path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bento_path here is to tell import_service
where to import the service, so it makes sense to return the parent dir of it.
This is to handle the case where bento_identifier
is a path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bento_path is used as the working directory of sub-processes. When bento_identifier is a absolute path, I'm not sure if module parent directory works for all the case. Thinking about
|
|- service/service.py
|- utils/some_utils.py
And in service.py, utils is imported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So normalize_identifier
is to return two values, import string and bento_path. For a bento identifier as absolute path. What shall it return? normalize_identifier("/path/to/package.py:Inference")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just said this case happens when user call service.serve_http()
, thus I think we should inherit the working directory, which is '.'
Will that work?
@frostming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but the first return value will be directly feed to import_service
. An absolute path isn't a valid import string right?
efd34b7
to
61e367a
Compare
For more information, see https://pre-commit.ci
Signed-off-by: Frost Ming [email protected]
What does this PR address?
This is the case when running
Service.serve_http()
Fixes #(issue)
Before submitting:
guide on how to create a pull request.
pre-commit run -a
script has passed (instructions)?those accordingly? Here are documentation guidelines and tips on writting docs.