From 01a889363c9560fd935d751e36eaf479ede46215 Mon Sep 17 00:00:00 2001 From: Hammad Bashir Date: Fri, 11 Oct 2024 05:59:29 -0700 Subject: [PATCH] [CLN] Return 409 for uniqueconstrainterrors (#2813) ## 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 --- chromadb/db/base.py | 7 ------- chromadb/db/impl/grpc/client.py | 3 +-- chromadb/db/mixins/sysdb.py | 10 ++-------- chromadb/errors.py | 11 +++++++++++ chromadb/test/db/test_system.py | 3 +-- clients/js/src/ChromaFetch.ts | 3 +++ clients/js/src/Errors.ts | 7 +++++++ clients/js/test/admin.test.ts | 4 ++-- 8 files changed, 27 insertions(+), 21 deletions(-) diff --git a/chromadb/db/base.py b/chromadb/db/base.py index 9427465d4ed..0abe62208e9 100644 --- a/chromadb/db/base.py +++ b/chromadb/db/base.py @@ -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.""" diff --git a/chromadb/db/impl/grpc/client.py b/chromadb/db/impl/grpc/client.py index ede6681f048..34ee63d01e6 100644 --- a/chromadb/db/impl/grpc/client.py +++ b/chromadb/db/impl/grpc/client.py @@ -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, diff --git a/chromadb/db/mixins/sysdb.py b/chromadb/db/mixins/sysdb.py index 4290754f33b..4127d3aa74b 100644 --- a/chromadb/db/mixins/sysdb.py +++ b/chromadb/db/mixins/sysdb.py @@ -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, diff --git a/chromadb/errors.py b/chromadb/errors.py index 808cce175da..4b0245d6855 100644 --- a/chromadb/errors.py +++ b/chromadb/errors.py @@ -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: diff --git a/chromadb/test/db/test_system.py b/chromadb/test/db/test_system.py index e89697c9d12..50bc83560fa 100644 --- a/chromadb/test/db/test_system.py +++ b/chromadb/test/db/test_system.py @@ -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 @@ -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 diff --git a/clients/js/src/ChromaFetch.ts b/clients/js/src/ChromaFetch.ts index ebd98cc15a9..b7373f9c14f 100644 --- a/clients/js/src/ChromaFetch.ts +++ b/clients/js/src/ChromaFetch.ts @@ -8,6 +8,7 @@ import { ChromaValueError, ChromaError, createErrorByType, + ChromaUniqueError, } from "./Errors"; import { FetchAPI } from "./generated"; @@ -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: diff --git a/clients/js/src/Errors.ts b/clients/js/src/Errors.ts index 0669968b4a5..d3a077cd9f9 100644 --- a/clients/js/src/Errors.ts +++ b/clients/js/src/Errors.ts @@ -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": diff --git a/clients/js/test/admin.test.ts b/clients/js/test/admin.test.ts index 0c7606d5f71..669209cc719 100644 --- a/clients/js/test/admin.test.ts +++ b/clients/js/test/admin.test.ts @@ -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", () => { @@ -79,7 +79,7 @@ describe("AdminClient", () => { expect(false).toBe(true); } catch (error) { expect(error).toBeInstanceOf(Error); - expect(error).toBeInstanceOf(ChromaError); + expect(error).toBeInstanceOf(ChromaUniqueError); } }); });