-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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.
So this essentially allows to call http://127.0.0.1:8321/
directly without v1
?
not directly. default is to include the version. on a per- |
@@ -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("/") |
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.
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.
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.
is the goal of this "prettification" of the URL?
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.
what issues do you anticipate?
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.
would a flexible prefix be a good option instead of boolean to set it v1
given the stability levels ?
What does this PR do?
it provides infrastructure for
@webmethod
endpoints that do not have / should not have a version prefix added, e.g. future openai endpoints.Test Plan
ci