Respond to review

Author: tsmith023

.github/workflows/main.yaml

@@ -164,14 +164,10 @@ jobs:
           WCS_DUMMY_CI_PW: ${{ secrets.WCS_DUMMY_CI_PW }}
           OKTA_DUMMY_CI_PW: ${{ secrets.OKTA_DUMMY_CI_PW }}
 #          OPENAI_APIKEY: ${{ secrets.OPENAI_APIKEY }}  disabled until we have a working key
-        run: |
-          pytest -n auto -m "not serial" --dist loadgroup -v --cov --cov-report=term-missing --cov=weaviate --cov-report xml:coverage-integration.xml integration
-          pytest -n0 -m "serial" -v --cov --cov-report=term-missing --cov=weaviate --cov-report xml:coverage-integration.xml integration
+        run: pytest -n auto --dist loadgroup -v --cov --cov-report=term-missing --cov=weaviate --cov-report xml:coverage-integration.xml integration
       - name: Run integration tests without auth secrets (for forks)
         if: ${{ github.event.pull_request.head.repo.fork }}
-        run: |
-          pytest -n auto -m "not serial" --dist loadgroup -v --cov --cov-report=term-missing --cov=weaviate --cov-report xml:coverage-integration.xml integration
-          pytest -n0 -m "serial" -v --cov --cov-report=term-missing --cov=weaviate --cov-report xml:coverage-integration.xml integration
+        run: pytest -n auto --dist loadgroup -v --cov --cov-report=term-missing --cov=weaviate --cov-report xml:coverage-integration.xml integration
       - name: Archive code coverage results
         if: matrix.versions.py == '3.10' && (github.ref_name != 'main')
         uses: actions/upload-artifact@v4

integration/test_backup_v4.py

@@ -7,7 +7,7 @@
 
 import weaviate
 import weaviate.classes as wvc
-from weaviate.backup.executor import (
+from weaviate.backup.backup import (
     BackupCompressionLevel,
     BackupConfigCreate,
     BackupConfigRestore,
@@ -117,7 +117,6 @@ def _create_backup_id() -> str:
     return str(round(time.time_ns() * 1000))
 
 
-@pytest.mark.serial
 def test_create_and_restore_backup_with_waiting(client: weaviate.WeaviateClient) -> None:
     """Create and restore backup with waiting."""
     backup_id = _create_backup_id()
@@ -156,7 +155,6 @@ def test_create_and_restore_backup_with_waiting(client: weaviate.WeaviateClient)
     assert restore_status.status == BackupStatus.SUCCESS
 
 
-@pytest.mark.serial
 @pytest.mark.parametrize("include", [["Article"], "Article"])
 def test_create_and_restore_backup_without_waiting(
     client: weaviate.WeaviateClient, include: Union[str, List[str]]
@@ -212,7 +210,6 @@ def test_create_and_restore_backup_without_waiting(
     assert len(client.collections.use("Article")) == len(ARTICLES_IDS)
 
 
-@pytest.mark.serial
 def test_create_and_restore_1_of_2_classes(client: weaviate.WeaviateClient) -> None:
     """Create and restore 1 of 2 classes."""
     backup_id = _create_backup_id()
@@ -248,7 +245,6 @@ def test_create_and_restore_1_of_2_classes(client: weaviate.WeaviateClient) -> N
     assert restore_status.status == BackupStatus.SUCCESS
 
 
-@pytest.mark.serial
 def test_fail_on_non_existing_class(client: weaviate.WeaviateClient) -> None:
     """Fail backup functions on non-existing class"""
     backup_id = _create_backup_id()
@@ -260,7 +256,6 @@ def test_fail_on_non_existing_class(client: weaviate.WeaviateClient) -> None:
             assert "422" in str(excinfo.value)
 
 
-@pytest.mark.serial
 def test_fail_restoring_backup_for_existing_class(client: weaviate.WeaviateClient) -> None:
     """Fail restoring backup for existing class."""
     backup_id = _create_backup_id()
@@ -285,7 +280,6 @@ def test_fail_restoring_backup_for_existing_class(client: weaviate.WeaviateClien
         assert "already exists" in str(excinfo.value)
 
 
-@pytest.mark.serial
 def test_fail_creating_existing_backup(client: weaviate.WeaviateClient) -> None:
     """Fail creating existing backup."""
     backup_id = _create_backup_id()
@@ -310,7 +304,6 @@ def test_fail_creating_existing_backup(client: weaviate.WeaviateClient) -> None:
         assert "422" in str(excinfo.value)
 
 
-@pytest.mark.serial
 def test_fail_restoring_non_existing_backup(client: weaviate.WeaviateClient) -> None:
     """Fail restoring non-existing backup."""
     backup_id = _create_backup_id()
@@ -320,7 +313,6 @@ def test_fail_restoring_non_existing_backup(client: weaviate.WeaviateClient) ->
         assert "404" in str(excinfo.value)
 
 
-@pytest.mark.serial
 def test_fail_checking_status_for_non_existing_restore(client: weaviate.WeaviateClient) -> None:
     """Fail checking restore status for non-existing restore."""
     backup_id = _create_backup_id()
@@ -334,7 +326,6 @@ def test_fail_checking_status_for_non_existing_restore(client: weaviate.Weaviate
             assert "404" in str(excinfo)
 
 
-@pytest.mark.serial
 def test_fail_creating_backup_for_both_include_and_exclude_classes(
     client: weaviate.WeaviateClient,
 ) -> None:
@@ -357,7 +348,6 @@ def test_fail_creating_backup_for_both_include_and_exclude_classes(
             )
 
 
-@pytest.mark.serial
 @pytest.mark.parametrize("dynamic_backup_location", [False, True])
 def test_backup_and_restore_with_collection(
     client: weaviate.WeaviateClient, dynamic_backup_location: bool, tmp_path: pathlib.Path
@@ -414,7 +404,6 @@ def test_backup_and_restore_with_collection(
     assert restore_status.status == BackupStatus.SUCCESS
 
 
-@pytest.mark.serial
 def test_backup_and_restore_with_collection_and_config_1_24_x(
     client: weaviate.WeaviateClient,
 ) -> None:
@@ -462,7 +451,6 @@ def test_backup_and_restore_with_collection_and_config_1_24_x(
     assert restore_status.status == BackupStatus.SUCCESS
 
 
-@pytest.mark.serial
 def test_backup_and_restore_with_collection_and_config_1_23_x(
     client: weaviate.WeaviateClient,
 ) -> None:
@@ -508,7 +496,6 @@ def test_backup_and_restore_with_collection_and_config_1_23_x(
 #     assert backup_id in [b.backup_id for b in backups]
 
 
-@pytest.mark.serial
 @pytest.mark.parametrize("dynamic_backup_location", [False, True])
 def test_cancel_backup(
     client: weaviate.WeaviateClient, dynamic_backup_location, tmp_path: pathlib.Path

integration/test_collection_get.py

@@ -8,6 +8,7 @@
 
 from integration.conftest import CollectionFactoryGet, CollectionFactory
 from weaviate.collections import Collection
+from weaviate.collections.data.sync import _DataCollection
 from weaviate.exceptions import InvalidDataModelException
 
 
@@ -26,16 +27,16 @@ class Right(TypedDict):
     assert isinstance(col, Collection)
 
 
-# def test_data_with_data_model_with_dict_generic(
-#     collection_factory_get: CollectionFactoryGet, request: SubRequest
-# ) -> None:
-#     class Right(TypedDict):
-#         name: str
+def test_data_with_data_model_with_dict_generic(
+    collection_factory_get: CollectionFactoryGet, request: SubRequest
+) -> None:
+    class Right(TypedDict):
+        name: str
 
-#     col = collection_factory_get(request.node.name)
-#     assert isinstance(col, Collection)
-#     data = col.data.with_data_model(Right)
-#     assert isinstance(data, _DataCollection)
+    col = collection_factory_get(request.node.name)
+    assert isinstance(col, Collection)
+    data = col.data.with_data_model(Right)
+    assert isinstance(data, _DataCollection)
 
 
 WRONG_PROPERTIES_ERROR_MSG = "properties can only be a dict type, e.g. Dict[str, Any], or a class that inherits from TypedDict"

pytest.ini

@@ -2,5 +2,4 @@
 addopts = -m 'not profiling' --benchmark-skip -l
 markers =
     profiling: marks tests that can be profiled
-    serial: marks tests that should be run serially
 asyncio_default_fixture_loop_scope = function

weaviate/backup/backup.py

@@ -0,0 +1,80 @@
+from enum import Enum
+from typing import Any, Dict, List, Optional, cast
+from pydantic import BaseModel, Field
+from weaviate.backup.backup_location import _BackupLocationConfig
+
+STORAGE_NAMES = {
+    "filesystem",
+    "s3",
+    "gcs",
+    "azure",
+}
+
+
+class BackupCompressionLevel(str, Enum):
+    """Which compression level should be used to compress the backup."""
+
+    DEFAULT = "DefaultCompression"
+    BEST_SPEED = "BestSpeed"
+    BEST_COMPRESSION = "BestCompression"
+
+
+class BackupStorage(str, Enum):
+    """Which backend should be used to write the backup to."""
+
+    FILESYSTEM = "filesystem"
+    S3 = "s3"
+    GCS = "gcs"
+    AZURE = "azure"
+
+
+class BackupStatus(str, Enum):
+    """The status of a backup."""
+
+    STARTED = "STARTED"
+    TRANSFERRING = "TRANSFERRING"
+    TRANSFERRED = "TRANSFERRED"
+    SUCCESS = "SUCCESS"
+    FAILED = "FAILED"
+    CANCELED = "CANCELED"
+
+
+class _BackupConfigBase(BaseModel):
+    CPUPercentage: Optional[int] = Field(default=None, alias="cpu_percentage")
+
+    def _to_dict(self) -> Dict[str, Any]:
+        ret = cast(dict, self.model_dump(exclude_none=True))
+
+        for key, val in ret.items():
+            if isinstance(val, _BackupLocationConfig):
+                ret[key] = val._to_dict()
+
+        return ret
+
+
+class BackupConfigCreate(_BackupConfigBase):
+    """Options to configure the backup when creating a backup."""
+
+    ChunkSize: Optional[int] = Field(default=None, alias="chunk_size")
+    CompressionLevel: Optional[BackupCompressionLevel] = Field(
+        default=None, alias="compression_level"
+    )
+
+
+class BackupConfigRestore(_BackupConfigBase):
+    """Options to configure the backup when restoring a backup."""
+
+
+class BackupStatusReturn(BaseModel):
+    """Return type of the backup status methods."""
+
+    error: Optional[str] = Field(default=None)
+    status: BackupStatus
+    path: str
+    backup_id: str = Field(alias="id")
+
+
+class BackupReturn(BackupStatusReturn):
+    """Return type of the backup creation and restore methods."""
+
+    collections: List[str] = Field(default_factory=list, alias="classes")

weaviate/backup/executor.py

@@ -4,13 +4,20 @@
 
 import asyncio
 import time
-from enum import Enum
-from typing import Generic, Optional, Union, List, Tuple, Dict, Any, cast
+from typing import Generic, Optional, Union, List, Tuple, Dict
 
 from httpx import Response
-from pydantic import BaseModel, Field
 
-from weaviate.backup.backup_location import _BackupLocationConfig, BackupLocationType
+from weaviate.backup.backup import (
+    BackupStorage,
+    BackupReturn,
+    BackupStatusReturn,
+    STORAGE_NAMES,
+    BackupConfigCreate,
+    BackupStatus,
+    BackupConfigRestore,
+)
+from weaviate.backup.backup_location import BackupLocationType
 from weaviate.connect import executor
 from weaviate.connect.v4 import _ExpectedStatusCodes, Connection, ConnectionAsync, ConnectionType
 from weaviate.exceptions import (
@@ -25,82 +32,6 @@
     _decode_json_response_dict,
 )
 
-STORAGE_NAMES = {
-    "filesystem",
-    "s3",
-    "gcs",
-    "azure",
-}
-
-
-class BackupCompressionLevel(str, Enum):
-    """Which compression level should be used to compress the backup."""
-
-    DEFAULT = "DefaultCompression"
-    BEST_SPEED = "BestSpeed"
-    BEST_COMPRESSION = "BestCompression"
-
-
-class BackupStorage(str, Enum):
-    """Which backend should be used to write the backup to."""
-
-    FILESYSTEM = "filesystem"
-    S3 = "s3"
-    GCS = "gcs"
-    AZURE = "azure"
-
-
-class BackupStatus(str, Enum):
-    """The status of a backup."""
-
-    STARTED = "STARTED"
-    TRANSFERRING = "TRANSFERRING"
-    TRANSFERRED = "TRANSFERRED"
-    SUCCESS = "SUCCESS"
-    FAILED = "FAILED"
-    CANCELED = "CANCELED"
-
-
-class _BackupConfigBase(BaseModel):
-    CPUPercentage: Optional[int] = Field(default=None, alias="cpu_percentage")
-
-    def _to_dict(self) -> Dict[str, Any]:
-        ret = cast(dict, self.model_dump(exclude_none=True))
-
-        for key, val in ret.items():
-            if isinstance(val, _BackupLocationConfig):
-                ret[key] = val._to_dict()
-
-        return ret
-
-
-class BackupConfigCreate(_BackupConfigBase):
-    """Options to configure the backup when creating a backup."""
-
-    ChunkSize: Optional[int] = Field(default=None, alias="chunk_size")
-    CompressionLevel: Optional[BackupCompressionLevel] = Field(
-        default=None, alias="compression_level"
-    )
-
-
-class BackupConfigRestore(_BackupConfigBase):
-    """Options to configure the backup when restoring a backup."""
-
-
-class BackupStatusReturn(BaseModel):
-    """Return type of the backup status methods."""
-
-    error: Optional[str] = Field(default=None)
-    status: BackupStatus
-    path: str
-    backup_id: str = Field(alias="id")
-
-
-class BackupReturn(BackupStatusReturn):
-    """Return type of the backup creation and restore methods."""
-
-    collections: List[str] = Field(default_factory=list, alias="classes")
-
 
 class _BackupExecutor(Generic[ConnectionType]):
     def __init__(self, connection: Connection):

weaviate/classes/backup.py

@@ -1,11 +1,10 @@
-from weaviate.backup.executor import (
+from weaviate.backup.backup import (
     BackupCompressionLevel,
     BackupConfigCreate,
     BackupConfigRestore,
     BackupStorage,
-    BackupLocationType,
 )
-from weaviate.backup.backup_location import BackupLocation
+from weaviate.backup.backup_location import BackupLocation, BackupLocationType
 
 
 __all__ = [

weaviate/client.pyi

@@ -4,15 +4,14 @@ Client class definition.
 
 from typing import Optional, Tuple, Union, Dict, Any
 
-from weaviate.backup.backup import _BackupAsync
-from weaviate.backup.sync import _Backup
 from weaviate.collections.classes.internal import _RawGQLReturn
 from weaviate.collections.collections.async_ import _CollectionsAsync
 from weaviate.collections.collections.sync import _Collections
 
 from weaviate.users.async_ import _UsersAsync
 
 from weaviate.users.sync import _Users
+from .backup import _Backup, _BackupAsync
 from .collections.batch.client import _BatchClientWrapper
 from .collections.cluster import _Cluster, _ClusterAsync
 from .connect.v4 import ConnectionV4