Skip to content

chore(api): make version prefix optional #3306

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(api): make version prefix optional #3306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion llama_stack/core/server/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ def get_all_api_routes(
# The __webmethod__ attribute is dynamically added by the @webmethod decorator
# mypy doesn't know about this dynamic attribute, so we ignore the attr-defined error
webmethod = method.__webmethod__ # type: ignore[attr-defined]
path = f"/{LLAMA_STACK_API_VERSION}/{webmethod.route.lstrip('/')}"
path = webmethod.route.lstrip("/")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is allowing v1 to be omitted something that is a norm? If so, I think this is nice as v1/openai/v1 is redundant. However, I am unsure if this Is something that'd play nicely with API stability down the line, or if this is something other project with API contracts regularly do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the goal of this "prettification" of the URL?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what issues do you anticipate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would a flexible prefix be a good option instead of boolean to set it v1 given the stability levels ?

if webmethod.add_version:
path = f"/{LLAMA_STACK_API_VERSION}/{path}"
if webmethod.method == hdrs.METH_GET:
http_method = hdrs.METH_GET
elif webmethod.method == hdrs.METH_DELETE:
Expand Down
3 changes: 3 additions & 0 deletions llama_stack/schema_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class WebMethod:
descriptive_name: str | None = None
experimental: bool | None = False
required_scope: str | None = None
add_version: bool = True


T = TypeVar("T", bound=Callable[..., Any])
Expand All @@ -38,6 +39,7 @@ def webmethod(
descriptive_name: str | None = None,
experimental: bool | None = False,
required_scope: str | None = None,
add_version: bool = True,
) -> Callable[[T], T]:
"""
Decorator that supplies additional metadata to an endpoint operation function.
Expand All @@ -61,6 +63,7 @@ def wrap(func: T) -> T:
descriptive_name=descriptive_name,
experimental=experimental,
required_scope=required_scope,
add_version=add_version,
)
return func

Expand Down
Loading