Skip to content

Commit fc9ccbd

Browse files
authored
Fix/cli: standardize exit codes, messages and streams (#592)
- Standardize non-MCP rvl CLI behavior: exit 0 for success/help, exit 2 for usage/argument and schema input issues, exit 1 for runtime failures; stderr for errors and stdout for normal output or help. - Catch specific exception types in the CLI to map to the right exit code (e.g. invalid path vs Redis search) - Version: print version to stdout (not only via logger).
1 parent 1335ed5 commit fc9ccbd

6 files changed

Lines changed: 154 additions & 50 deletions

File tree

redisvl/cli/index.py

Lines changed: 77 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
import sys
33
from argparse import Namespace
44

5+
import yaml
6+
from pydantic import ValidationError
7+
58
from redisvl.cli.utils import add_index_parsing_options, create_redis_url
9+
from redisvl.exceptions import RedisSearchError
610
from redisvl.index import SearchIndex
711
from redisvl.redis.connection import RedisConnectionFactory
812
from redisvl.redis.utils import convert_bytes, make_dict
@@ -11,6 +15,36 @@
1115

1216
logger = get_logger("[RedisVL]")
1317

18+
# Exceptions commonly raised when loading or validating a schema path (-s).
19+
SCHEMA_INPUT_ERRORS = (
20+
FileNotFoundError,
21+
ValueError,
22+
yaml.YAMLError,
23+
ValidationError,
24+
)
25+
26+
27+
def exit_schema_input_error(args: Namespace, exc: BaseException) -> None:
28+
if not args.schema:
29+
raise exc
30+
print(str(exc), file=sys.stderr)
31+
sys.exit(2)
32+
33+
34+
def exit_redis_search_error(
35+
args: Namespace, index: SearchIndex | None, exc: RedisSearchError
36+
) -> None:
37+
name = (
38+
index.schema.index.name
39+
if index is not None
40+
else (args.index or args.schema or "unknown")
41+
)
42+
print(
43+
f"Redis search operation failed for index {name!r}. {exc}",
44+
file=sys.stderr,
45+
)
46+
sys.exit(1)
47+
1448

1549
class Index:
1650
usage = "\n".join(
@@ -33,26 +67,38 @@ def __init__(self):
3367
parser = add_index_parsing_options(parser)
3468

3569
args = parser.parse_args(sys.argv[2:])
70+
3671
if not hasattr(self, args.command):
37-
parser.print_help()
38-
exit(0)
72+
print(f"Unknown command: {args.command}\n", file=sys.stderr)
73+
parser.print_help(sys.stderr)
74+
sys.exit(2)
75+
3976
try:
4077
getattr(self, args.command)(args)
4178
except Exception as e:
42-
logger.error(e)
43-
exit(0)
79+
logger.error(e, exc_info=True)
80+
print(str(e), file=sys.stderr)
81+
sys.exit(1)
4482

4583
def create(self, args: Namespace):
4684
"""Create an index.
4785
4886
Usage:
49-
rvl index create -i <index_name> | -s <schema_path>
87+
rvl index create -s <schema_path>
5088
"""
5189
if not args.schema:
52-
print("Schema must be provided to create an index")
53-
exit(1)
54-
index = SearchIndex.from_yaml(args.schema, redis_url=create_redis_url(args))
55-
index.create()
90+
print("Schema must be provided to create an index", file=sys.stderr)
91+
sys.exit(2)
92+
93+
redis_url = create_redis_url(args)
94+
try:
95+
index = SearchIndex.from_yaml(args.schema, redis_url=redis_url)
96+
except SCHEMA_INPUT_ERRORS as e:
97+
exit_schema_input_error(args, e)
98+
try:
99+
index.create()
100+
except RedisSearchError as e:
101+
exit_redis_search_error(args, index, e)
56102
print("Index created successfully")
57103

58104
def info(self, args: Namespace):
@@ -62,7 +108,10 @@ def info(self, args: Namespace):
62108
rvl index info -i <index_name> | -s <schema_path>
63109
"""
64110
index = self._connect_to_index(args)
65-
_display_in_table(index.info())
111+
try:
112+
_display_in_table(index.info())
113+
except RedisSearchError as e:
114+
exit_redis_search_error(args, index, e)
66115

67116
def listall(self, args: Namespace):
68117
"""List all indices.
@@ -84,7 +133,10 @@ def delete(self, args: Namespace, drop=False):
84133
rvl index delete -i <index_name> | -s <schema_path>
85134
"""
86135
index = self._connect_to_index(args)
87-
index.delete(drop=drop)
136+
try:
137+
index.delete(drop=drop)
138+
except RedisSearchError as e:
139+
exit_redis_search_error(args, index, e)
88140
print("Index deleted successfully")
89141

90142
def destroy(self, args: Namespace):
@@ -96,19 +148,23 @@ def destroy(self, args: Namespace):
96148
self.delete(args, drop=True)
97149

98150
def _connect_to_index(self, args: Namespace) -> SearchIndex:
99-
# connect to redis
100151
redis_url = create_redis_url(args)
101152

102153
if args.index:
103-
schema = IndexSchema.from_dict({"index": {"name": args.index}})
104-
index = SearchIndex(schema=schema, redis_url=redis_url)
105-
elif args.schema:
106-
index = SearchIndex.from_yaml(args.schema, redis_url=redis_url)
107-
else:
108-
print("Index name or schema must be provided")
109-
exit(1)
110-
111-
return index
154+
try:
155+
schema = IndexSchema.from_dict({"index": {"name": args.index}})
156+
return SearchIndex(schema=schema, redis_url=redis_url)
157+
except SCHEMA_INPUT_ERRORS as e:
158+
exit_schema_input_error(args, e)
159+
160+
if args.schema:
161+
try:
162+
return SearchIndex.from_yaml(args.schema, redis_url=redis_url)
163+
except SCHEMA_INPUT_ERRORS as e:
164+
exit_schema_input_error(args, e)
165+
166+
print("Index name or schema must be provided", file=sys.stderr)
167+
sys.exit(2)
112168

113169

114170
def _display_in_table(index_info):

redisvl/cli/main.py

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,36 @@ def __init__(self):
3030
parser.add_argument("command", help="Subcommand to run")
3131

3232
if len(sys.argv) < 2:
33-
parser.print_help()
34-
exit(0)
33+
parser.print_help(sys.stdout)
34+
sys.exit(0)
3535

3636
args = parser.parse_args(sys.argv[1:2])
37+
3738
if not hasattr(self, args.command):
38-
parser.print_help()
39-
exit(0)
40-
getattr(self, args.command)()
39+
print(f"Unknown command: {args.command}\n", file=sys.stderr)
40+
parser.print_help(sys.stderr)
41+
sys.exit(2)
42+
43+
try:
44+
getattr(self, args.command)()
45+
except Exception as e:
46+
print(e, file=sys.stderr)
47+
sys.exit(1)
4148

4249
def index(self):
4350
Index()
44-
exit(0)
51+
sys.exit(0)
4552

4653
def mcp(self):
4754
from redisvl.cli.mcp import MCP
4855

4956
MCP()
50-
exit(0)
57+
sys.exit(0)
5158

5259
def version(self):
5360
Version()
54-
exit(0)
61+
sys.exit(0)
5562

5663
def stats(self):
5764
Stats()
58-
exit(0)
65+
sys.exit(0)

redisvl/cli/mcp.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77

88

99
class _MCPArgumentParser(argparse.ArgumentParser):
10-
"""ArgumentParser variant that reports usage errors with exit code 1."""
10+
"""ArgumentParser variant that reports usage errors with exit code 2."""
1111

1212
def error(self, message):
1313
self.print_usage(sys.stderr)
14-
self.exit(1, "%s: error: %s\n" % (self.prog, message))
14+
self.exit(2, "%s: error: %s\n" % (self.prog, message))
1515

1616

1717
class MCP:

redisvl/cli/stats.py

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,48 @@
22
import sys
33
from argparse import Namespace
44

5+
import yaml
6+
from pydantic import ValidationError
7+
58
from redisvl.cli.utils import add_index_parsing_options, create_redis_url
9+
from redisvl.exceptions import RedisSearchError
610
from redisvl.index import SearchIndex
711
from redisvl.schema.schema import IndexSchema
812
from redisvl.utils.log import get_logger
913

1014
logger = get_logger("[RedisVL]")
1115

16+
# Exceptions commonly raised when loading or validating a schema path (-s).
17+
SCHEMA_INPUT_ERRORS = (
18+
FileNotFoundError,
19+
ValueError,
20+
yaml.YAMLError,
21+
ValidationError,
22+
)
23+
24+
25+
def exit_schema_input_error(args: Namespace, exc: BaseException) -> None:
26+
if not args.schema:
27+
raise exc
28+
print(str(exc), file=sys.stderr)
29+
sys.exit(2)
30+
31+
32+
def exit_redis_search_error(
33+
args: Namespace, index: SearchIndex | None, exc: RedisSearchError
34+
) -> None:
35+
name = (
36+
index.schema.index.name
37+
if index is not None
38+
else (args.index or args.schema or "unknown")
39+
)
40+
print(
41+
f"Redis search operation failed for index {name!r}. {exc}",
42+
file=sys.stderr,
43+
)
44+
sys.exit(1)
45+
46+
1247
STATS_KEYS = [
1348
"num_docs",
1449
"num_terms",
@@ -43,11 +78,13 @@ def __init__(self):
4378
parser = argparse.ArgumentParser(usage=self.usage)
4479
parser = add_index_parsing_options(parser)
4580
args = parser.parse_args(sys.argv[2:])
81+
4682
try:
4783
self.stats(args)
4884
except Exception as e:
49-
logger.error(e)
50-
exit(0)
85+
logger.error(e, exc_info=True)
86+
print(str(e), file=sys.stderr)
87+
sys.exit(1)
5188

5289
def stats(self, args: Namespace):
5390
"""Obtain stats about an index.
@@ -56,22 +93,29 @@ def stats(self, args: Namespace):
5693
rvl stats -i <index_name> | -s <schema_path>
5794
"""
5895
index = self._connect_to_index(args)
59-
_display_stats(index.info())
96+
try:
97+
_display_stats(index.info())
98+
except RedisSearchError as e:
99+
exit_redis_search_error(args, index, e)
60100

61101
def _connect_to_index(self, args: Namespace) -> SearchIndex:
62-
# connect to redis
63102
redis_url = create_redis_url(args)
64103

65104
if args.index:
66-
schema = IndexSchema.from_dict({"index": {"name": args.index}})
67-
index = SearchIndex(schema=schema, redis_url=redis_url)
68-
elif args.schema:
69-
index = SearchIndex.from_yaml(args.schema, redis_url=redis_url)
70-
else:
71-
logger.error("Index name or schema must be provided")
72-
exit(0)
73-
74-
return index
105+
try:
106+
schema = IndexSchema.from_dict({"index": {"name": args.index}})
107+
return SearchIndex(schema=schema, redis_url=redis_url)
108+
except SCHEMA_INPUT_ERRORS as e:
109+
exit_schema_input_error(args, e)
110+
111+
if args.schema:
112+
try:
113+
return SearchIndex.from_yaml(args.schema, redis_url=redis_url)
114+
except SCHEMA_INPUT_ERRORS as e:
115+
exit_schema_input_error(args, e)
116+
117+
print("Index name or schema must be provided", file=sys.stderr)
118+
sys.exit(2)
75119

76120

77121
def _display_stats(index_info):

redisvl/cli/version.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@
33
from argparse import Namespace
44

55
from redisvl import __version__
6-
from redisvl.utils.log import get_logger
7-
8-
logger = get_logger("[RedisVL]")
96

107

118
class Version:
@@ -29,4 +26,4 @@ def version(self, args: Namespace):
2926
if args.short:
3027
print(__version__)
3128
else:
32-
logger.info(f"RedisVL version {__version__}")
29+
print(f"RedisVL version {__version__}")

tests/unit/test_cli_mcp.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ def test_mcp_command_rejects_invalid_transport(monkeypatch, capsys):
464464
with pytest.raises(SystemExit) as exc_info:
465465
module.MCP()
466466

467-
assert exc_info.value.code == 1
467+
assert exc_info.value.code == 2
468468
out = capsys.readouterr()
469469
assert "invalid choice" in out.err or "invalid choice" in out.out
470470

0 commit comments

Comments
 (0)