Merge pull request #402 from lfnovo/fix/model-duplicate-check-include-type
fix: include type in model duplicate check
This commit is contained in:
commit
62730a9486
2 changed files with 28 additions and 6 deletions
|
|
@ -98,16 +98,16 @@ async def create_model(model_data: ModelCreate):
|
|||
detail=f"Invalid model type. Must be one of: {valid_types}"
|
||||
)
|
||||
|
||||
# Check for duplicate model name under the same provider (case-insensitive)
|
||||
# Check for duplicate model name under the same provider and type (case-insensitive)
|
||||
from open_notebook.database.repository import repo_query
|
||||
existing = await repo_query(
|
||||
"SELECT * FROM model WHERE string::lowercase(provider) = $provider AND string::lowercase(name) = $name LIMIT 1",
|
||||
{"provider": model_data.provider.lower(), "name": model_data.name.lower()}
|
||||
"SELECT * FROM model WHERE string::lowercase(provider) = $provider AND string::lowercase(name) = $name AND string::lowercase(type) = $type LIMIT 1",
|
||||
{"provider": model_data.provider.lower(), "name": model_data.name.lower(), "type": model_data.type.lower()}
|
||||
)
|
||||
if existing:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Model '{model_data.name}' already exists for provider '{model_data.provider}'"
|
||||
detail=f"Model '{model_data.name}' already exists for provider '{model_data.provider}' with type '{model_data.type}'"
|
||||
)
|
||||
|
||||
new_model = Model(
|
||||
|
|
|
|||
|
|
@ -41,7 +41,7 @@ class TestModelCreation:
|
|||
assert response.status_code == 400
|
||||
assert (
|
||||
response.json()["detail"]
|
||||
== "Model 'gpt-4' already exists for provider 'openai'"
|
||||
== "Model 'gpt-4' already exists for provider 'openai' with type 'language'"
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -70,7 +70,7 @@ class TestModelCreation:
|
|||
assert response.status_code == 400
|
||||
assert (
|
||||
response.json()["detail"]
|
||||
== "Model 'GPT-4' already exists for provider 'OpenAI'"
|
||||
== "Model 'GPT-4' already exists for provider 'OpenAI' with type 'language'"
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -95,6 +95,28 @@ class TestModelCreation:
|
|||
# Should succeed because provider is different
|
||||
assert response.status_code == 200
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("open_notebook.database.repository.repo_query")
|
||||
async def test_create_same_model_name_different_type(
|
||||
self, mock_repo_query, client
|
||||
):
|
||||
"""Test that creating a model with same name but different type is allowed."""
|
||||
from open_notebook.ai.models import Model
|
||||
|
||||
# Mock repo_query to return empty (no duplicate found for different type)
|
||||
mock_repo_query.return_value = []
|
||||
|
||||
# Patch the save method on the Model class
|
||||
with patch.object(Model, "save", new_callable=AsyncMock) as mock_save:
|
||||
# Attempt to create same model name with different type (embedding instead of language)
|
||||
response = client.post(
|
||||
"/api/models",
|
||||
json={"name": "gpt-4", "provider": "openai", "type": "embedding"},
|
||||
)
|
||||
|
||||
# Should succeed because type is different
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
class TestModelsProviderAvailability:
|
||||
"""Test suite for Models Provider Availability endpoint."""
|
||||
|
|
|
|||
Loading…
Reference in a new issue