-
Notifications
You must be signed in to change notification settings - Fork 162
feat(BA-1607): Add runtime_variant parameter to Service creation SDK function #4749
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
Add a new runtime_variant
option to the Service creation functions in both the SDK and CLI, allowing callers to specify which runtime environment should back their inference service.
- Introduce
runtime_variant
parameter in the async client SDKcreate
method (type, docstring, and request payload) - Add a
--runtime-variant
click option to the CLIcreate
command and include it in the request body
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/ai/backend/client/func/service.py | Added optional runtime_variant param, docstring entry, and payload field |
src/ai/backend/client/cli/service.py | Imported RuntimeVariant , added --runtime-variant CLI option, and passed it in the request body |
Comments suppressed due to low confidence (2)
src/ai/backend/client/func/service.py:116
- There are no tests covering the new
runtime_variant
parameter. Add unit or integration tests to verify behavior when it is provided or omitted.
runtime_variant: Optional[str] = None,
src/ai/backend/client/func/service.py:1
- Missing import for
RuntimeVariant
, which is referenced in the type annotation suggestion and should also be used in the payload for consistency. Add:
from ai.backend.common.types import RuntimeVariant
from collections.abc import Mapping, Sequence
@@ -113,6 +113,7 @@ async def create( | |||
owner_access_key: Optional[str] = None, | |||
model_definition_path: Optional[str] = None, | |||
expose_to_public: bool = False, | |||
runtime_variant: Optional[str] = None, |
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.
Consider using the RuntimeVariant
enum for the runtime_variant
parameter instead of a plain string. For example:
from ai.backend.common.types import RuntimeVariant
...
runtime_variant: Optional[RuntimeVariant] = None,
runtime_variant: Optional[str] = None, | |
runtime_variant: Optional[RuntimeVariant] = None, |
Copilot uses AI. Check for mistakes.
src/ai/backend/client/cli/service.py
Outdated
@click.option( | ||
"--runtime-variant", | ||
metavar="RUNTIME_VARIANT", | ||
type=click.Choice([*RuntimeVariant], case_sensitive=False), |
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.
The click.Choice
invocation is passing the enum class directly. It should pass the enum values, e.g.:
type=click.Choice([rv.value for rv in RuntimeVariant], case_sensitive=False)
type=click.Choice([*RuntimeVariant], case_sensitive=False), | |
type=click.Choice([rv.value for rv in RuntimeVariant], case_sensitive=False), |
Copilot uses AI. Check for mistakes.
resolves #4748 (BA-1607)
Checklist: (if applicable)
ai.backend.test
docs
directory