Skip to content

Commit

Permalink
Merge pull request #214 from Cornerstone-OnDemand/213-librarysettings…
Browse files Browse the repository at this point in the history
…-fails-to-instantiate-when-modelkit_cache_provider-is-set-except-with-redis-provider

Fix settings of cache provider by environment variable
  • Loading branch information
antoinejeannot authored Feb 2, 2024
2 parents 6bc74ca + 9b979bc commit 830989c
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 13 deletions.
18 changes: 7 additions & 11 deletions modelkit/core/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,15 @@ class NativeCacheSettings(CacheSettings):

def cache_settings():
s = CacheSettings()
if s.cache_provider is None:

if s.cache_provider == "none":
return None
try:
elif s.cache_provider == "redis":
return RedisSettings()
except pydantic.ValidationError:
pass
try:
elif s.cache_provider == "native":
return NativeCacheSettings()
except pydantic.ValidationError:
pass
else:
return None


def _get_library_settings_cache_provider(v: Optional[str]) -> str:
Expand Down Expand Up @@ -134,8 +133,5 @@ class LibrarySettings(ModelkitSettings):
Annotated[None, pydantic.Tag("none")],
],
pydantic.Discriminator(_get_library_settings_cache_provider),
] = pydantic.Field(
default_factory=cache_settings,
union_mode="left_to_right",
)
] = pydantic.Field(default_factory=cache_settings)
model_config = pydantic.ConfigDict(extra="allow")
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ addopts = """
--failed-first
--durations 10
--color=yes
tests"""
"""

[tool.black]
target-version = ['py38']
Expand Down
28 changes: 27 additions & 1 deletion tests/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
from modelkit.assets.drivers.gcs import GCSStorageDriverSettings
from modelkit.assets.drivers.local import LocalStorageDriverSettings
from modelkit.assets.drivers.s3 import S3StorageDriverSettings
from modelkit.core.settings import ModelkitSettings
from modelkit.core.settings import (
LibrarySettings,
ModelkitSettings,
NativeCacheSettings,
RedisSettings,
)


def test_modelkit_settings_working(monkeypatch):
Expand Down Expand Up @@ -48,3 +53,24 @@ def test_storage_driver_settings(Settings, monkeypatch):
assert Settings(bucket="bar").bucket == "bar"
with pytest.raises(pydantic.ValidationError):
_ = Settings()


def test_cache_provider_settings(monkeypatch):
monkeypatch.setenv("MODELKIT_CACHE_PROVIDER", "redis")
lib_settings = LibrarySettings()
assert isinstance(lib_settings.cache, RedisSettings)
assert lib_settings.cache.cache_provider == "redis"

monkeypatch.setenv("MODELKIT_CACHE_PROVIDER", "native")
lib_settings = LibrarySettings()
assert isinstance(lib_settings.cache, NativeCacheSettings)
assert lib_settings.cache.cache_provider == "native"

monkeypatch.setenv("MODELKIT_CACHE_PROVIDER", "none")
assert LibrarySettings().cache is None

monkeypatch.setenv("MODELKIT_CACHE_PROVIDER", "not supported")
assert LibrarySettings().cache is None

monkeypatch.delenv("MODELKIT_CACHE_PROVIDER")
assert LibrarySettings().cache is None

0 comments on commit 830989c

Please sign in to comment.