Skip to content

Commit

Permalink
[CLN] Return 409 for uniqueconstrainterrors (#2813)
Browse files Browse the repository at this point in the history
## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
	 - Returns 409 for uniqueconstrainterrors
 - New functionality
	 - None

## Test plan
*How are these changes tested?*
- [x] Tests pass locally with `pytest` for python, `yarn test` for js,
`cargo test` for rust

## Documentation Changes
None
  • Loading branch information
HammadB authored Oct 11, 2024
1 parent fcf9db0 commit 01a8893
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 21 deletions.
7 changes: 0 additions & 7 deletions chromadb/db/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,9 @@
from chromadb.config import System, Component
from uuid import UUID
from itertools import islice, count

from chromadb.types import SeqId


class UniqueConstraintError(Exception):
"""Raised when an insert operation would violate a unique constraint"""

pass


class Cursor(Protocol):
"""Reifies methods we use from a DBAPI2 Cursor since DBAPI2 is not typed."""

Expand Down
3 changes: 1 addition & 2 deletions chromadb/db/impl/grpc/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
from overrides import overrides
from chromadb.api.configuration import CollectionConfigurationInternal
from chromadb.config import DEFAULT_DATABASE, DEFAULT_TENANT, System, logger
from chromadb.db.base import UniqueConstraintError
from chromadb.db.system import SysDB
from chromadb.errors import NotFoundError
from chromadb.errors import NotFoundError, UniqueConstraintError
from chromadb.proto.convert import (
from_proto_collection,
from_proto_segment,
Expand Down
10 changes: 2 additions & 8 deletions chromadb/db/mixins/sysdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,9 @@
InvalidConfigurationError,
)
from chromadb.config import DEFAULT_DATABASE, DEFAULT_TENANT, System
from chromadb.db.base import (
Cursor,
SqlDB,
ParameterValue,
get_sql,
UniqueConstraintError,
)
from chromadb.db.base import Cursor, SqlDB, ParameterValue, get_sql
from chromadb.db.system import SysDB
from chromadb.errors import NotFoundError
from chromadb.errors import NotFoundError, UniqueConstraintError
from chromadb.telemetry.opentelemetry import (
add_attributes_to_current_span,
OpenTelemetryClient,
Expand Down
11 changes: 11 additions & 0 deletions chromadb/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,17 @@ def name(cls) -> str:
return "NotFoundError"


class UniqueConstraintError(ChromaError):
@overrides
def code(self) -> int:
return 409

@classmethod
@overrides
def name(cls) -> str:
return "UniqueConstraintError"


class BatchSizeExceededError(ChromaError):
@overrides
def code(self) -> int:
Expand Down
3 changes: 1 addition & 2 deletions chromadb/test/db/test_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from chromadb.db.impl.grpc.client import GrpcSysDB
from chromadb.db.impl.grpc.server import GrpcMockSysDB
from chromadb.errors import NotFoundError
from chromadb.errors import NotFoundError, UniqueConstraintError
from chromadb.test.conftest import find_free_port
from chromadb.types import Collection, Segment, SegmentScope
from chromadb.db.impl.sqlite import SqliteDB
Expand All @@ -17,7 +17,6 @@
Settings,
)
from chromadb.db.system import SysDB
from chromadb.db.base import UniqueConstraintError
from pytest import FixtureRequest
import uuid
from chromadb.api.configuration import CollectionConfigurationInternal
Expand Down
3 changes: 3 additions & 0 deletions clients/js/src/ChromaFetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
ChromaValueError,
ChromaError,
createErrorByType,
ChromaUniqueError,
} from "./Errors";
import { FetchAPI } from "./generated";

Expand Down Expand Up @@ -71,6 +72,8 @@ export const chromaFetch: FetchAPI = async (
throw new ChromaNotFoundError(
`The requested resource could not be found: ${input}`,
);
case 409:
throw new ChromaUniqueError("The resource already exists");
case 500:
throw parseServerError(respBody?.error);
case 502:
Expand Down
7 changes: 7 additions & 0 deletions clients/js/src/Errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ export class InvalidCollectionError extends Error {
}
}

export class ChromaUniqueError extends Error {
name = "ChromaUniqueError";
constructor(message: string, public readonly cause?: unknown) {
super(message);
}
}

export function createErrorByType(type: string, message: string) {
switch (type) {
case "InvalidCollection":
Expand Down
4 changes: 2 additions & 2 deletions clients/js/test/admin.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterAll, beforeAll, describe, expect, test } from "@jest/globals";
import { AdminClient } from "../src/AdminClient";
import { ChromaError } from "../src/Errors";
import { ChromaError, ChromaUniqueError } from "../src/Errors";
import { startChromaContainer } from "./startChromaContainer";

describe("AdminClient", () => {
Expand Down Expand Up @@ -79,7 +79,7 @@ describe("AdminClient", () => {
expect(false).toBe(true);
} catch (error) {
expect(error).toBeInstanceOf(Error);
expect(error).toBeInstanceOf(ChromaError);
expect(error).toBeInstanceOf(ChromaUniqueError);
}
});
});

0 comments on commit 01a8893

Please sign in to comment.