added remote_base_domain as an optional param #975
Conversation
anupreet
changed the title
truss/cli/cli.py
Outdated
| @@ -226,6 +226,12 @@ def run(target_directory: str, build_dir: Path, tag, port, attach) -> None: | |||
| required=False, | |||
| help="Name of the remote in .trussrc to patch changes to", | |||
| ) | |||
| @click.option( | |||
| @@ -24,11 +24,19 @@ | |||
| remote_provider=test_remote | |||
| """ | |||
|
|
|||
| SAMPLE_TRUSSRC_WITH_REMOTE_BASE_DOMAIN = """ | |||
| [test] | |||
There was a problem hiding this comment.
this is missing
remote_provider = baseten
truss/cli/cli.py
Outdated
| @@ -464,6 +470,12 @@ def _extract_request_data(data: Optional[str], file: Optional[Path]): | |||
| required=False, | |||
| help="Name of the remote in .trussrc to push to", | |||
| ) | |||
| @click.option( | |||
There was a problem hiding this comment.
we don't need these extra CLI commands. The base domain will automatically get pulled out from remote.
truss/cli/cli.py
Outdated
| @@ -510,6 +522,7 @@ def _extract_request_data(data: Optional[str], file: Optional[Path]): | |||
| def predict( | |||
| target_directory: str, | |||
| remote: str, | |||
| remote_base_domain: str, | |||
There was a problem hiding this comment.
covert by your revert comment, but this won't be needed.
truss/remote/baseten/remote.py
Outdated
| self, | ||
| remote_url: str, | ||
| api_key: str, | ||
| remote_base_domain: Optional[str] = None, |
There was a problem hiding this comment.
I wonder if we should rename this to invocation_domain. remote_base seems indistinguishable from the remote_url so it's somewhat confusing.
There was a problem hiding this comment.
agreed that remote and remote_base_domain are too similar, one nit would be to have this be predict_domain just to be consistent w/ the name of the endpoint
There was a problem hiding this comment.
yeah.. I think "base domain" covers the "xyz.com" may be remote_inference_base_domain might be more accurate...
truss/remote/baseten/service.py
Outdated
| url = _add_model_domain( | ||
| self._api.rest_api_url, f"model-{self.model_id}", self.remote_base_domain | ||
| ) | ||
| print(url) |
anupreet
force-pushed
the
aw-add-domain-truss
branch
from
befca55 to
4190940
Compare
anupreet
force-pushed
the
aw-add-domain-truss
branch
from
4190940 to
7a7e4ab
Compare
🚀 What
Added a way to set a domain for predict calls for Baseten.
If remote_base_domain is set in the truss.rc then it adds this to the predict url before making the inference request
💻 How
Added a way to configure the remote_base_domain through cli and through truss.rc
🔬 Testing
Tested it locally!
