Skip to content
This repository was archived by the owner on Sep 22, 2023. It is now read-only.

Commit b6a2bd9

Browse files
authored
fix: Handling default values in JSONParamType (#180)
* Click invokes ParamType.convert() method *twice* for string default values, which is unfortunately indistinguishable by the type as the official doc says when the user passes a signle JSON string literal.
1 parent be7c174 commit b6a2bd9

File tree

4 files changed

+23
-5
lines changed

4 files changed

+23
-5
lines changed

changes/180.fix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix handling of default values for JSON string CLI arugments (`client.cli.params.JSONParamType`)

src/ai/backend/client/cli/admin/scaling_group.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,11 @@ def list(ctx: CLIContext) -> None:
7575
help='New scaling group will be inactive.')
7676
@click.option('--driver', type=str, default='static',
7777
help='Set driver.')
78-
@click.option('--driver-opts', type=JSONParamType(), default={},
78+
@click.option('--driver-opts', type=JSONParamType(), default='{}',
7979
help='Set driver options as a JSON string.')
8080
@click.option('--scheduler', type=str, default='fifo',
8181
help='Set scheduler.')
82-
@click.option('--scheduler-opts', type=JSONParamType(), default={},
82+
@click.option('--scheduler-opts', type=JSONParamType(), default='{}',
8383
help='Set scheduler options as a JSON string.')
8484
def add(name, description, inactive,
8585
driver, driver_opts, scheduler, scheduler_opts):

src/ai/backend/client/cli/params.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,31 @@ def convert(self, value, param, ctx):
5555

5656

5757
class JSONParamType(click.ParamType):
58+
"""
59+
A JSON string parameter type.
60+
The default value must be given as a valid JSON-parsable string,
61+
not the Python objects.
62+
"""
63+
5864
name = "json-string"
5965

66+
def __init__(self) -> None:
67+
super().__init__()
68+
self._parsed = False
69+
6070
def convert(
6171
self,
62-
value: str,
72+
value: Optional[str],
6373
param: Optional[click.Parameter],
6474
ctx: Optional[click.Context],
6575
) -> Any:
76+
if self._parsed:
77+
# Click invokes this method TWICE
78+
# for a default value given as string.
79+
return value
80+
self._parsed = True
81+
if value is None:
82+
return None
6683
try:
6784
return json.loads(value)
6885
except json.JSONDecodeError:

src/ai/backend/client/func/scaling_group.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,9 @@ async def update(cls, name: str, description: str = '', is_active: bool = True,
164164
'description': description,
165165
'is_active': is_active,
166166
'driver': driver,
167-
'driver_opts': json.dumps(driver_opts),
167+
'driver_opts': None if driver_opts is None else json.dumps(driver_opts),
168168
'scheduler': scheduler,
169-
'scheduler_opts': json.dumps(scheduler_opts),
169+
'scheduler_opts': None if scheduler_opts is None else json.dumps(scheduler_opts),
170170
},
171171
}
172172
data = await api_session.get().Admin._query(query, variables)

0 commit comments

Comments
 (0)