diff --git a/doc/changelog.md b/doc/changelog.md index 1665c069f..814a16944 100644 --- a/doc/changelog.md +++ b/doc/changelog.md @@ -26,6 +26,7 @@ Description - Add instructions for installing SmartSim on PML's Scylla - Drop unsued development dependencies - Fix typos in documentation +- Fix `smart info` error on initial install Detailed Notes @@ -108,6 +109,12 @@ Detailed Notes - Removes an Numpy upper bound in the SmartSim dependency list now that SmartRedis and supported ML backends support Numpy 2.0. ([SmartSim-PR803](https://github.com/CrayLabs/SmartSim/pull/803)) +- SmartSim's `smart info` CLI tool would raise a spurious error on initial + install due to a missing build directory that is created as part of `smart + build`. Running `smart build` would fix this error. This error has now been + suppressed if the build directory is not present and `smart info` works as + intended prior to running `smart build`. + ([SmartSim-PR806](https://github.com/CrayLabs/SmartSim/pull/806)) ### 0.8.0 diff --git a/smartsim/_core/_cli/utils.py b/smartsim/_core/_cli/utils.py index 44a668b6e..5a61ba645 100644 --- a/smartsim/_core/_cli/utils.py +++ b/smartsim/_core/_cli/utils.py @@ -30,7 +30,7 @@ import subprocess as sp import sys from argparse import ArgumentParser, Namespace -from collections.abc import Callable +from collections.abc import Callable, Collection from pathlib import Path from smartsim._core._install.buildenv import SetupError @@ -118,12 +118,30 @@ def clean(core_path: Path, _all: bool = False) -> int: return os.EX_OK -def get_db_path() -> Path | None: - bin_path = get_install_path() / "_core" / "bin" - for option in bin_path.iterdir(): - if option.name in ("redis-cli", "keydb-cli"): - return option - return None +def get_db_path( + *, + bin_dir: os.PathLike[str] | None = None, + names: Collection[str] = ("redis-cli", "keydb-cli"), +) -> Path | None: + """Returns the path to an installed database CLI if it can find it in a + binary directory. If it cannot be found for any reason (binary directory + doesn't exist, file does not match expected name, etc.) `None` is returned. + If many candidates are found, only the path to the first found is returned. + + :param bin_path: The path to the binary directory to search for CLIs. If + none is provided, the default SmartSim binary directory will be + searched. Keyword only. + :returns: A path to the installed database CLI if found + """ + bin_path = ( + (get_install_path() / "_core" / "bin") if bin_dir is None else Path(bin_dir) + ) + cli_paths = ( + path + for name in names + if (path := bin_path / name).exists() and not path.is_dir() + ) + return next(cli_paths, None) _CliHandler = Callable[[Namespace, list[str]], int] diff --git a/tests/test_cli.py b/tests/test_cli.py index 63ad910ff..6e2dd24a9 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -25,9 +25,12 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import argparse +import itertools import logging import os import pathlib +import string +import textwrap import typing as t from collections import defaultdict from contextlib import contextmanager @@ -43,7 +46,7 @@ from smartsim._core._cli.clean import execute_all as clobber_execute from smartsim._core._cli.dbcli import execute as dbcli_execute from smartsim._core._cli.site import execute as site_execute -from smartsim._core._cli.utils import MenuItemConfig +from smartsim._core._cli.utils import MenuItemConfig, get_db_path from smartsim._core._cli.validate import configure_parser as validate_parser from smartsim._core._cli.validate import execute as validate_execute @@ -839,3 +842,98 @@ def test_validate_correctly_sets_and_restores_env(monkeypatch): assert os.environ["SPAM"] == "EGGS" assert "TICK" not in os.environ assert "DNE" not in os.environ + + +def test_get_db_path_for_dir_that_dne(test_dir): + dne_dir = pathlib.Path(test_dir) / "some/dir/that/dne" + assert get_db_path(bin_dir=dne_dir) == None + + +def test_get_db_path_raises_if_not_searching_dir(test_dir): + some_file = pathlib.Path(test_dir) / "some-file" + some_file.touch() + assert get_db_path(bin_dir=some_file) == None + + +def test_get_db_path_for_dir_that_is_empty(test_dir): + empty_dir = pathlib.Path(test_dir) / "empty" + empty_dir.mkdir() + assert get_db_path(bin_dir=empty_dir) == None + + +@pytest.mark.parametrize( + "candidate, should_find", + itertools.chain( + [ + pytest.param(file, True, id=f"find-{file}") + for file in ("redis-cli", "keydb-cli") + ], + [ + pytest.param(file, False, id=f"ignore-{file}") + for file in ( + "other-cli", + "random-file", + "redis-server", # This method is slightly misnamed, but IT IS SUPPOSED + "keydb-server", # to return the CLI and not the actual server + ) + ], + ), +) +def test_get_db_path_gets_expected_canidate_path(test_dir, candidate, should_find): + bin_dir = pathlib.Path(test_dir) / "bin" + bin_dir.mkdir() + file_path = bin_dir / candidate + file_path.touch() + file_path.write_text(textwrap.dedent("""\ + #!/usr/bin/env bash + echo 'Hello from the mock exe' + """)) + file_path.chmod(0o766) + assert get_db_path(bin_dir=bin_dir) == (file_path if should_find else None) + + +@pytest.mark.parametrize( + "target, extras", + [ + pytest.param("redis-cli", ["redis-server"], id="redis install"), + pytest.param("keydb-cli", ["keydb-server"], id="keydb install"), + pytest.param( + "redis-cli", + ["redis-server", "keydb-cli", "keydb-server"], + id="redis+keydb install", + ), + pytest.param( + "redis-cli", [f"file-{i}" for i in range(10)], id="lots of clutter" + ), + ], +) +def test_get_db_path_finds_file_in_crowded_dir(test_dir, target, extras): + bin_dir = pathlib.Path(test_dir) / "bin" + bin_dir.mkdir() + + for file in map(lambda name: bin_dir / name, (target, *extras)): + file.touch() + file.chmod(0o766) + assert get_db_path(bin_dir=bin_dir) == bin_dir / target + + +def test_get_db_path_does_not_return_a_directory(test_dir): + bin_dir = pathlib.Path(test_dir) / "bin" + bin_dir.mkdir() + target = bin_dir / "redis-cli" + target.mkdir() + assert get_db_path(bin_dir=bin_dir) == None + target.rmdir() + target.touch() + assert get_db_path(bin_dir=bin_dir) == target + + +def test_get_db_path_does_not_search_reciursive(test_dir): + bin_dir = pathlib.Path(test_dir) / "bin" + bin_dir.mkdir() + nested_dir = bin_dir / "nested" + nested_dir.mkdir() + target = nested_dir / "redis-cli" + target.touch() + assert get_db_path(bin_dir=nested_dir) == target + assert get_db_path(bin_dir=bin_dir) == None