Skip to content

Commit

Permalink
Fix settings_dict (#257)
Browse files Browse the repository at this point in the history
Error out if settings_dict is provided with other parameters in create_index
  • Loading branch information
wanliAlex authored Sep 17, 2024
1 parent 02d2010 commit 9d807e8
Show file tree
Hide file tree
Showing 12 changed files with 216 additions and 66 deletions.
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"tox"
],
name="marqo",
version="3.8.0",
version="3.8.1",
author="marqo org",
author_email="org@marqo.io",
description="Tensor search for humans",
Expand Down
1 change: 0 additions & 1 deletion src/marqo/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ def create(config: Config,
textChunkPrefix=text_chunk_prefix,
textQueryPrefix=text_query_prefix,
)

return req.post(f"indexes/{index_name}", body=local_create_index_settings.generate_request_body())

# py-marqo against Marqo Cloud
Expand Down
19 changes: 13 additions & 6 deletions src/marqo/models/create_index_settings.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from typing import Dict, Any, Optional, List

from pydantic import model_validator

from marqo.models import marqo_index
from pydantic import root_validator, Field
from marqo.models.marqo_models import MarqoBaseModel


Expand Down Expand Up @@ -56,11 +57,17 @@ def generate_request_body(self) -> dict:
if self.settingsDict is not None:
return self.settingsDict
else:
return self.dict(exclude_none=True, exclude={"settingsDict"})
return self.model_dump(exclude_none=True, exclude={"settingsDict"})

@root_validator(pre=True)
@model_validator(mode='before')
def check_settings_dict_compatibility(cls, values):
""" Ensures that settingsDict is not specified along with other parameters."""
if values.get("settings_dict") is not None and any(arg_name for arg_name in values):
raise ValueError(f"settings_dict cannot be specified with other index creation parameters.")
""" Ensures that settingsDict is not specified along with other parameters.
Raises:
ValueError: If settingsDict is specified along with other parameters.
"""
non_none_values = {k: v for k, v in values.items() if v is not None and k != "settingsDict"}
if values.get("settingsDict") is not None and len(non_none_values) > 0:
raise ValueError(f"'settings_dict' cannot be specified with other index creation parameters. You can "
f"move the parameters {list(non_none_values.keys())} to settingsDict.")
return values
6 changes: 2 additions & 4 deletions src/marqo/models/search_models.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
from typing import Dict, List, Optional, Union
from marqo.models.marqo_models import StrictBaseModel
from abc import ABC
from enum import Enum
from typing import Dict, List, Optional, Union

from pydantic import validator, BaseModel, root_validator
from marqo.models.marqo_models import StrictBaseModel


class SearchBody(StrictBaseModel):
Expand Down
1 change: 0 additions & 1 deletion tests/cloud_test_logic/cloud_test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,5 +135,4 @@ class CloudTestIndex(str, Enum):
"tensorFields": ["multimodal_field", "text_field_3", "video_field_3", "audio_field_2", "image_field_2"],
"normalizeEmbeddings": True,
},

}
2 changes: 1 addition & 1 deletion tests/cloud_test_logic/run_cloud_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def convert_string_to_boolean(string_value):
sys.exit(1)
print(f"All indices has been created, proceeding to run tests with pytest. Arguments: {sys.argv[1:]}")

pytest_args = ['tests/', '-m', 'not ignore_during_cloud_tests'] + sys.argv[1:]
pytest_args = ['tests/', '--cloud'] + sys.argv[1:]
print("running integration tests with args:", pytest_args)
pytest_exit_code = pytest.main(pytest_args)
if pytest_exit_code != 0:
Expand Down
44 changes: 35 additions & 9 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,47 @@ def pytest_addoption(parser):
# TODO Remove this after all the tests are fixed
# This is only used as a temporary measure to run all tests
# If not specified, only tests marked as 'fixed' will be run
parser.addoption("--all", action="store_true", help="run all tests")
parser.addoption("--all", action="store_true", default=False, help="run all tests")
parser.addoption("--cloud", action="store_true", default=False, help="run pytest in the cloud mode. Skip tests"
"marked as 'local_only_tests'.")
parser.addoption("--local", action="store_true", default=True, help="run pytest in the local mode. Skip tests"
"marked as 'cloud_only_tests'.")


def pytest_collection_modifyitems(config, items):
# If --all option is specified, run all tests
if config.getoption("--all"):
# If the --all option is used, run all tests
return
else:
# Skip tests not marked as 'fixed'
skip_non_fixed = pytest.mark.skip(reason="not marked as fixed")
for item in items:
# TODO Remove this after all the tests are fixed
if "fixed" not in item.keywords:
item.add_marker(skip_non_fixed)

# Skip tests not marked as 'fixed'
skip_non_fixed = pytest.mark.skip(reason="not marked as fixed")
# Skip tests marked as 'cloud_only_tests' when in local mode
skip_cloud_only = pytest.mark.skip(reason="skipped in local mode")
# Skip tests marked as 'local_only_tests' when in cloud mode
skip_local_only = pytest.mark.skip(reason="skipped in cloud mode")

# If --cloud is provided, set --local to False
if config.getoption("--cloud"):
config.option.local = False

for item in items:
if "fixed" not in item.keywords:
item.add_marker(skip_non_fixed)

# If running in cloud mode, skip local-only tests
if config.getoption("--cloud"):
if "local_only_tests" in item.keywords:
item.add_marker(skip_local_only)

# If running in local mode, skip cloud-only tests
if config.getoption("--local"):
if "cloud_only_tests" in item.keywords:
item.add_marker(skip_cloud_only)


def pytest_configure(config):
# Register custom markers programmatically
config.addinivalue_line("markers", "local_only_tests: mark a test to run only in local mode")
config.addinivalue_line("markers", "cloud_only_tests: mark a test to run only in cloud mode")
config.addinivalue_line("markers", "fixed: mark test to run as part of fixed tests")

2 changes: 1 addition & 1 deletion tests/marqo_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class Config:
arbitrary_types_allowed: bool = True

def __str__(self):
return f"MockHTTPTraffic({json.dumps(self.dict(), indent=2)})"
return f"MockHTTPTraffic({json.dumps(self.model_dump(), indent=2)})"

def raise_(ex):
raise ex
Expand Down
Loading

0 comments on commit 9d807e8

Please sign in to comment.