Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove RedisStorage #4156

Merged
merged 3 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions .github/workflows/tests-storage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,6 @@ jobs:
OMP_NUM_THREADS: 1
TEST_DB_URL: postgresql+psycopg2://user:test@127.0.0.1/optunatest

- name: Tests Redis
run: |
pytest tests/storages_tests/test_with_server.py
env:
OMP_NUM_THREADS: 1
TEST_DB_URL: redis://localhost:6379

- name: Tests Journal Redis
run: |
pytest tests/storages_tests/test_with_server.py
Expand Down
4 changes: 1 addition & 3 deletions asv.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@
// pip (with all the conda available packages installed first,
// followed by the pip installed packages).
//
"matrix": {
"fakeredis": [""],
},
"matrix": {},

// Combinations of libraries/python versions can be excluded/included
// from the set to test. Each entry is a dictionary containing additional
Expand Down
3 changes: 0 additions & 3 deletions benchmarks/asv/optimize.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ def time_optimize(self, args: str) -> None:
"inmemory, cmaes, 1000",
"sqlite, random, 1000",
"cached_sqlite, random, 1000",
# Following benchmarks use fakeredis instead of Redis.
"redis, random, 1000",
"cached_redis, random, 1000",
)
param_names = ["storage, sampler, n_trials"]
timeout = 600
1 change: 0 additions & 1 deletion docs/source/reference/storages.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ The :mod:`~optuna.storages` module defines a :class:`~optuna.storages.BaseStorag
:nosignatures:

optuna.storages.RDBStorage
optuna.storages.RedisStorage
optuna.storages.RetryFailedTrialCallback
optuna.storages.fail_stale_trials
optuna.storages.JournalStorage
Expand Down
4 changes: 3 additions & 1 deletion optuna/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,9 @@ def take_action(self, parsed_args: Namespace) -> None:

storage_url = _check_storage_url(self.app_args.storage)
if storage_url.startswith("redis"):
self.logger.info("This storage does not support upgrade yet.")
self.logger.info(
"RedisStorage is removed at Optuna v3.1.0. Please use JournalRedisStorage instead."
)
c-bata marked this conversation as resolved.
Show resolved Hide resolved
return
storage = RDBStorage(storage_url, skip_compatibility_check=True, skip_table_creation=True)
current_version = storage.get_current_version()
Expand Down
3 changes: 0 additions & 3 deletions optuna/integration/allennlp/_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,6 @@ def __init__(
if isinstance(storage, optuna.storages.RDBStorage):
url = storage.url

elif isinstance(storage, optuna.storages.RedisStorage):
url = storage._url

elif isinstance(storage, optuna.storages._CachedStorage):
assert isinstance(storage._backend, optuna.storages.RDBStorage)
url = storage._backend.url
Expand Down
12 changes: 5 additions & 7 deletions optuna/storages/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@
from optuna.storages._journal.redis import JournalRedisStorage
from optuna.storages._journal.storage import JournalStorage
from optuna.storages._rdb.storage import RDBStorage
from optuna.storages._redis import RedisStorage


__all__ = [
"BaseStorage",
"BaseJournalLogStorage",
"InMemoryStorage",
"RDBStorage",
"RedisStorage",
"JournalStorage",
"JournalFileSymlinkLock",
"JournalFileOpenLock",
Expand All @@ -38,11 +36,11 @@ def get_storage(storage: Union[None, str, BaseStorage]) -> BaseStorage:
if storage is None:
return InMemoryStorage()
if isinstance(storage, str):
if storage.startswith("redis"):
return _CachedStorage(RedisStorage(storage))
else:
return _CachedStorage(RDBStorage(storage))
elif isinstance(storage, (RDBStorage, RedisStorage)):
assert not storage.startswith(
c-bata marked this conversation as resolved.
Show resolved Hide resolved
"redis"
), "RedisStorage is removed at Optuna v3.1.0. Please use JournalRedisStorage instead."
HideakiImamura marked this conversation as resolved.
Show resolved Hide resolved
return _CachedStorage(RDBStorage(storage))
elif isinstance(storage, RDBStorage):
return _CachedStorage(storage)
else:
return storage
9 changes: 3 additions & 6 deletions optuna/storages/_cached_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from optuna.storages import BaseStorage
from optuna.storages._heartbeat import BaseHeartbeat
from optuna.storages._rdb.storage import RDBStorage
from optuna.storages._redis import RedisStorage
from optuna.study._frozen import FrozenStudy
from optuna.study._study_direction import StudyDirection
from optuna.trial import FrozenTrial
Expand All @@ -39,8 +38,7 @@ class _CachedStorage(BaseStorage, BaseHeartbeat):
"""A wrapper class of storage backends.

This class is used in :func:`~optuna.get_storage` function and automatically
wraps :class:`~optuna.storages.RDBStorage` class or
:class:`~optuna.storages.RedisStorage` class.
wraps :class:`~optuna.storages.RDBStorage` class.

:class:`~optuna.storages._CachedStorage` meets the following **Consistency** and
**Data persistence** requirements.
Expand Down Expand Up @@ -79,11 +77,10 @@ class _CachedStorage(BaseStorage, BaseHeartbeat):

Args:
backend:
:class:`~optuna.storages.RDBStorage` class or :class:`~optuna.storages.RedisStorage`
class instance to wrap.
:class:`~optuna.storages.RDBStorage` class instance to wrap.
"""

def __init__(self, backend: Union[RDBStorage, RedisStorage]) -> None:
def __init__(self, backend: RDBStorage) -> None:
self._backend = backend
self._studies: Dict[int, _StudyInfo] = {}
self._trial_id_to_study_id_and_number: Dict[int, Tuple[int, int]] = {}
Expand Down
Loading