forked from phoenix-oss/llama-stack-mirror
feat(api): don't return a payload on file delete (#1640)
# What does this PR do? This is to stay consistent with other APIs. This change registers files in API, even though there are still no providers. Removing tests that require a provider existing for a merged API to enable it in API layer. Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com> [//]: # (If resolving an issue, uncomment and update the line below) [//]: # (Closes #[issue-number]) ## Test Plan [Describe the tests you ran to verify your changes with result summaries. *Provide clear instructions so the plan can be easily re-executed.*] [//]: # (## Documentation) Signed-off-by: Ihar Hrachyshka <ihar.hrachyshka@gmail.com>
This commit is contained in:
parent
65d5d0d1bf
commit
367c08f01e
9 changed files with 143 additions and 119 deletions
89
docs/_static/llama-stack-spec.html
vendored
89
docs/_static/llama-stack-spec.html
vendored
|
@ -818,14 +818,7 @@
|
|||
"delete": {
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "OK",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/FileResponse"
|
||||
}
|
||||
}
|
||||
}
|
||||
"description": "OK"
|
||||
},
|
||||
"400": {
|
||||
"$ref": "#/components/responses/BadRequest400"
|
||||
|
@ -6140,46 +6133,6 @@
|
|||
"title": "FileUploadResponse",
|
||||
"description": "Response after initiating a file upload session."
|
||||
},
|
||||
"FileResponse": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"bucket": {
|
||||
"type": "string",
|
||||
"description": "Bucket under which the file is stored (valid chars: a-zA-Z0-9_-)"
|
||||
},
|
||||
"key": {
|
||||
"type": "string",
|
||||
"description": "Key under which the file is stored (valid chars: a-zA-Z0-9_-/.)"
|
||||
},
|
||||
"mime_type": {
|
||||
"type": "string",
|
||||
"description": "MIME type of the file"
|
||||
},
|
||||
"url": {
|
||||
"type": "string",
|
||||
"description": "Upload URL for the file contents"
|
||||
},
|
||||
"bytes": {
|
||||
"type": "integer",
|
||||
"description": "Size of the file in bytes"
|
||||
},
|
||||
"created_at": {
|
||||
"type": "integer",
|
||||
"description": "Timestamp of when the file was created"
|
||||
}
|
||||
},
|
||||
"additionalProperties": false,
|
||||
"required": [
|
||||
"bucket",
|
||||
"key",
|
||||
"mime_type",
|
||||
"url",
|
||||
"bytes",
|
||||
"created_at"
|
||||
],
|
||||
"title": "FileResponse",
|
||||
"description": "Response representing a file entry."
|
||||
},
|
||||
"EmbeddingsRequest": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
@ -6933,6 +6886,46 @@
|
|||
"title": "URIDataSource",
|
||||
"description": "A dataset that can be obtained from a URI."
|
||||
},
|
||||
"FileResponse": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"bucket": {
|
||||
"type": "string",
|
||||
"description": "Bucket under which the file is stored (valid chars: a-zA-Z0-9_-)"
|
||||
},
|
||||
"key": {
|
||||
"type": "string",
|
||||
"description": "Key under which the file is stored (valid chars: a-zA-Z0-9_-/.)"
|
||||
},
|
||||
"mime_type": {
|
||||
"type": "string",
|
||||
"description": "MIME type of the file"
|
||||
},
|
||||
"url": {
|
||||
"type": "string",
|
||||
"description": "Upload URL for the file contents"
|
||||
},
|
||||
"bytes": {
|
||||
"type": "integer",
|
||||
"description": "Size of the file in bytes"
|
||||
},
|
||||
"created_at": {
|
||||
"type": "integer",
|
||||
"description": "Timestamp of when the file was created"
|
||||
}
|
||||
},
|
||||
"additionalProperties": false,
|
||||
"required": [
|
||||
"bucket",
|
||||
"key",
|
||||
"mime_type",
|
||||
"url",
|
||||
"bytes",
|
||||
"created_at"
|
||||
],
|
||||
"title": "FileResponse",
|
||||
"description": "Response representing a file entry."
|
||||
},
|
||||
"Model": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
|
70
docs/_static/llama-stack-spec.yaml
vendored
70
docs/_static/llama-stack-spec.yaml
vendored
|
@ -557,10 +557,6 @@ paths:
|
|||
responses:
|
||||
'200':
|
||||
description: OK
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: '#/components/schemas/FileResponse'
|
||||
'400':
|
||||
$ref: '#/components/responses/BadRequest400'
|
||||
'429':
|
||||
|
@ -4286,39 +4282,6 @@ components:
|
|||
title: FileUploadResponse
|
||||
description: >-
|
||||
Response after initiating a file upload session.
|
||||
FileResponse:
|
||||
type: object
|
||||
properties:
|
||||
bucket:
|
||||
type: string
|
||||
description: >-
|
||||
Bucket under which the file is stored (valid chars: a-zA-Z0-9_-)
|
||||
key:
|
||||
type: string
|
||||
description: >-
|
||||
Key under which the file is stored (valid chars: a-zA-Z0-9_-/.)
|
||||
mime_type:
|
||||
type: string
|
||||
description: MIME type of the file
|
||||
url:
|
||||
type: string
|
||||
description: Upload URL for the file contents
|
||||
bytes:
|
||||
type: integer
|
||||
description: Size of the file in bytes
|
||||
created_at:
|
||||
type: integer
|
||||
description: Timestamp of when the file was created
|
||||
additionalProperties: false
|
||||
required:
|
||||
- bucket
|
||||
- key
|
||||
- mime_type
|
||||
- url
|
||||
- bytes
|
||||
- created_at
|
||||
title: FileResponse
|
||||
description: Response representing a file entry.
|
||||
EmbeddingsRequest:
|
||||
type: object
|
||||
properties:
|
||||
|
@ -4830,6 +4793,39 @@ components:
|
|||
title: URIDataSource
|
||||
description: >-
|
||||
A dataset that can be obtained from a URI.
|
||||
FileResponse:
|
||||
type: object
|
||||
properties:
|
||||
bucket:
|
||||
type: string
|
||||
description: >-
|
||||
Bucket under which the file is stored (valid chars: a-zA-Z0-9_-)
|
||||
key:
|
||||
type: string
|
||||
description: >-
|
||||
Key under which the file is stored (valid chars: a-zA-Z0-9_-/.)
|
||||
mime_type:
|
||||
type: string
|
||||
description: MIME type of the file
|
||||
url:
|
||||
type: string
|
||||
description: Upload URL for the file contents
|
||||
bytes:
|
||||
type: integer
|
||||
description: Size of the file in bytes
|
||||
created_at:
|
||||
type: integer
|
||||
description: Timestamp of when the file was created
|
||||
additionalProperties: false
|
||||
required:
|
||||
- bucket
|
||||
- key
|
||||
- mime_type
|
||||
- url
|
||||
- bytes
|
||||
- created_at
|
||||
title: FileResponse
|
||||
description: Response representing a file entry.
|
||||
Model:
|
||||
type: object
|
||||
properties:
|
||||
|
|
|
@ -21,7 +21,7 @@ from llama_stack.distribution.stack import LlamaStack # noqa: E402
|
|||
|
||||
from .pyopenapi.options import Options # noqa: E402
|
||||
from .pyopenapi.specification import Info, Server # noqa: E402
|
||||
from .pyopenapi.utility import Specification, validate_api_method_return_types # noqa: E402
|
||||
from .pyopenapi.utility import Specification, validate_api # noqa: E402
|
||||
|
||||
|
||||
def str_presenter(dumper, data):
|
||||
|
@ -40,8 +40,7 @@ def main(output_dir: str):
|
|||
raise ValueError(f"Directory {output_dir} does not exist")
|
||||
|
||||
# Validate API protocols before generating spec
|
||||
print("Validating API method return types...")
|
||||
return_type_errors = validate_api_method_return_types()
|
||||
return_type_errors = validate_api()
|
||||
if return_type_errors:
|
||||
print("\nAPI Method Return Type Validation Errors:\n")
|
||||
for error in return_type_errors:
|
||||
|
|
|
@ -7,10 +7,9 @@
|
|||
import json
|
||||
import typing
|
||||
import inspect
|
||||
import os
|
||||
from pathlib import Path
|
||||
from typing import TextIO
|
||||
from typing import Any, Dict, List, Optional, Protocol, Type, Union, get_type_hints, get_origin, get_args
|
||||
from typing import Any, List, Optional, Union, get_type_hints, get_origin, get_args
|
||||
|
||||
from llama_stack.strong_typing.schema import object_to_json, StrictJsonType
|
||||
from llama_stack.distribution.resolver import api_protocol_map
|
||||
|
@ -125,29 +124,59 @@ def is_optional_type(type_: Any) -> bool:
|
|||
return origin is Optional or (origin is Union and type(None) in args)
|
||||
|
||||
|
||||
def validate_api_method_return_types() -> List[str]:
|
||||
"""Validate that all API methods have proper return types."""
|
||||
def _validate_api_method_return_type(method) -> str | None:
|
||||
hints = get_type_hints(method)
|
||||
|
||||
if 'return' not in hints:
|
||||
return "has no return type annotation"
|
||||
|
||||
return_type = hints['return']
|
||||
if is_optional_type(return_type):
|
||||
return "returns Optional type"
|
||||
|
||||
|
||||
def _validate_api_delete_method_returns_none(method) -> str | None:
|
||||
hints = get_type_hints(method)
|
||||
|
||||
if 'return' not in hints:
|
||||
return "has no return type annotation"
|
||||
|
||||
return_type = hints['return']
|
||||
if return_type is not None and return_type is not type(None):
|
||||
return "does not return None"
|
||||
|
||||
|
||||
_VALIDATORS = {
|
||||
"GET": [
|
||||
_validate_api_method_return_type,
|
||||
],
|
||||
"DELETE": [
|
||||
_validate_api_delete_method_returns_none,
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
def _get_methods_by_type(protocol, method_type: str):
|
||||
members = inspect.getmembers(protocol, predicate=inspect.isfunction)
|
||||
return {
|
||||
method_name: method
|
||||
for method_name, method in members
|
||||
if (webmethod := getattr(method, '__webmethod__', None))
|
||||
if webmethod and webmethod.method == method_type
|
||||
}
|
||||
|
||||
|
||||
def validate_api() -> List[str]:
|
||||
"""Validate the API protocols."""
|
||||
errors = []
|
||||
protocols = api_protocol_map()
|
||||
|
||||
for protocol_name, protocol in protocols.items():
|
||||
methods = inspect.getmembers(protocol, predicate=inspect.isfunction)
|
||||
|
||||
for method_name, method in methods:
|
||||
if not hasattr(method, '__webmethod__'):
|
||||
continue
|
||||
|
||||
# Only check GET methods
|
||||
if method.__webmethod__.method != "GET":
|
||||
continue
|
||||
|
||||
hints = get_type_hints(method)
|
||||
|
||||
if 'return' not in hints:
|
||||
errors.append(f"Method {protocol_name}.{method_name} has no return type annotation")
|
||||
else:
|
||||
return_type = hints['return']
|
||||
if is_optional_type(return_type):
|
||||
errors.append(f"Method {protocol_name}.{method_name} returns Optional type")
|
||||
for target, validators in _VALIDATORS.items():
|
||||
for protocol_name, protocol in protocols.items():
|
||||
for validator in validators:
|
||||
for method_name, method in _get_methods_by_type(protocol, target).items():
|
||||
err = validator(method)
|
||||
if err:
|
||||
errors.append(f"Method {protocol_name}.{method_name} {err}")
|
||||
|
||||
return errors
|
||||
|
|
|
@ -34,6 +34,7 @@ class Api(Enum):
|
|||
scoring_functions = "scoring_functions"
|
||||
benchmarks = "benchmarks"
|
||||
tool_groups = "tool_groups"
|
||||
files = "files"
|
||||
|
||||
# built-in API
|
||||
inspect = "inspect"
|
||||
|
|
|
@ -164,7 +164,7 @@ class Files(Protocol):
|
|||
self,
|
||||
bucket: str,
|
||||
key: str,
|
||||
) -> FileResponse:
|
||||
) -> None:
|
||||
"""
|
||||
Delete a file identified by a bucket and key.
|
||||
|
||||
|
|
|
@ -12,6 +12,7 @@ from llama_stack.apis.benchmarks import Benchmarks
|
|||
from llama_stack.apis.datasetio import DatasetIO
|
||||
from llama_stack.apis.datasets import Datasets
|
||||
from llama_stack.apis.eval import Eval
|
||||
from llama_stack.apis.files import Files
|
||||
from llama_stack.apis.inference import Inference
|
||||
from llama_stack.apis.inspect import Inspect
|
||||
from llama_stack.apis.models import Models
|
||||
|
@ -79,6 +80,7 @@ def api_protocol_map() -> Dict[Api, Any]:
|
|||
Api.post_training: PostTraining,
|
||||
Api.tool_groups: ToolGroups,
|
||||
Api.tool_runtime: ToolRuntime,
|
||||
Api.files: Files,
|
||||
}
|
||||
|
||||
|
||||
|
|
11
llama_stack/providers/registry/files.py
Normal file
11
llama_stack/providers/registry/files.py
Normal file
|
@ -0,0 +1,11 @@
|
|||
# Copyright (c) Meta Platforms, Inc. and affiliates.
|
||||
# All rights reserved.
|
||||
#
|
||||
# This source code is licensed under the terms described in the LICENSE file in
|
||||
# the root directory of this source tree.
|
||||
|
||||
from llama_stack.providers.datatypes import ProviderSpec
|
||||
|
||||
|
||||
def available_providers() -> list[ProviderSpec]:
|
||||
return []
|
|
@ -14,17 +14,10 @@ from llama_stack.distribution.utils.dynamic import instantiate_class_type
|
|||
class TestProviderConfigurations:
|
||||
"""Test suite for testing provider configurations across all API types."""
|
||||
|
||||
def test_all_api_providers_exist(self):
|
||||
provider_registry = get_provider_registry()
|
||||
for api in providable_apis():
|
||||
providers = provider_registry.get(api, {})
|
||||
assert providers, f"No providers found for API type: {api}"
|
||||
|
||||
@pytest.mark.parametrize("api", providable_apis())
|
||||
def test_api_providers(self, api):
|
||||
provider_registry = get_provider_registry()
|
||||
providers = provider_registry.get(api, {})
|
||||
assert providers, f"No providers found for API type: {api}"
|
||||
|
||||
failures = []
|
||||
for provider_type, provider_spec in providers.items():
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue