Skip to content

Commit

Permalink
Merge pull request #1121 from procrastinate-org/fix-args-shell-1095
Browse files Browse the repository at this point in the history
  • Loading branch information
ewjoachim authored Jul 24, 2024
2 parents e94b4d0 + 7ccaf5c commit 8e0fde2
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 8 deletions.
8 changes: 4 additions & 4 deletions procrastinate/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ def configure_shell_parser(subparsers: argparse._SubParsersAction):
)
add_argument(
shell_parser,
"args",
"shell_command",
nargs="*",
help="Invoke a shell command and exit",
)
Expand Down Expand Up @@ -644,16 +644,16 @@ async def healthchecks(app: procrastinate.App):
print("Found procrastinate_jobs table: OK")


async def shell_(app: procrastinate.App, args: list[str]):
async def shell_(app: procrastinate.App, shell_command: list[str]):
"""
Administration shell for procrastinate.
"""
shell_obj = shell.ProcrastinateShell(
job_manager=app.job_manager,
)

if args:
await utils.sync_to_async(shell_obj.onecmd, line=shlex.join(args))
if shell_command:
await utils.sync_to_async(shell_obj.onecmd, line=shlex.join(shell_command))
else:
await utils.sync_to_async(shell_obj.cmdloop)

Expand Down
36 changes: 36 additions & 0 deletions tests/unit/contrib/django/test_cli.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
from __future__ import annotations

import argparse

from procrastinate.contrib.django.management.commands import (
procrastinate as procrastinate_command,
)


def test_procrastinate_command():
# This test might be brittle as it uses internal argparse attributes
# It's linked to https://github.com/procrastinate-org/procrastinate/pull/1095
parser = procrastinate_command.Command().create_parser("manage.py", "procrastinate")

error = """
If this test fails, it means that an argument named "args" has been added
(or renamed) in the procrastinate CLI, and is exposed in the Django procrastinate
management command. This is problematic because "args" is a special name that
Django removes from the command line arguments before passing them to the
management command. To fix this error, use a different name for the argument.
See:
- https://github.com/django/django/blob/f9bf616597d56deac66d9d6bb753b028dd9634cc/django/core/management/base.py#L410
- https://github.com/procrastinate-org/procrastinate/pull/1095
"""

def assert_no_action_named_args(parser):
for action in parser._actions:
assert getattr(action, "dest", "") != (
"args"
), f"'args' found in {parser.prog}\n{error}"
if isinstance(action, argparse._SubParsersAction):
for subparser in action.choices.values():
assert_no_action_named_args(subparser)

assert_no_action_named_args(parser)
8 changes: 4 additions & 4 deletions tests/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ def test_main(mocker):
["shell"],
{
"command": "shell",
"args": [],
"shell_command": [],
},
),
(
["shell", "list_jobs"],
{
"command": "shell",
"args": ["list_jobs"],
"shell_command": ["list_jobs"],
},
),
],
Expand Down Expand Up @@ -288,7 +288,7 @@ def mytask(a):

await mytask.defer_async(a=1)

await cli.shell_(app=app, args=["list_jobs"])
await cli.shell_(app=app, shell_command=["list_jobs"])

out, _ = capsys.readouterr()

Expand All @@ -304,7 +304,7 @@ def mytask(a):

mocker.patch("sys.stdin", io.StringIO("list_jobs\nexit\n"))

await cli.shell_(app=app, args=[])
await cli.shell_(app=app, shell_command=[])

out, _ = capsys.readouterr()

Expand Down

0 comments on commit 8e0fde2

Please sign in to comment.