Skip to content

Commit 672c91c

Browse files
committed
šŸ‘Œ verdi presto: only treat AIIDA_NO_DAEMON_AUTOSTART=1 as truthy
The previous implementation treated any non-empty value of `AIIDA_NO_DAEMON_AUTOSTART` as "skip daemon". That made `AIIDA_NO_DAEMON_AUTOSTART=0` and `=false` silently skip the daemon, contrary to what those values read as. The env var is an internal escape hatch for the Docker init script (and similar CI setups), not a public API surface. Keep the recognized form narrow: only the literal string `1` skips the daemon. Any other value (`0`, `true`, `false`, empty, typos) falls through to "start the daemon", which is the safe default. Drop the env var from user-facing reference: `--help` and `command_line.rst` no longer mention it. Documenting it in the CLI reference would turn an internal hatch into public API and require a deprecation cycle to remove. The Docker init script already comments why it sets the var; that's the right place for consumer-side docs. A short comment near the call site in `cmd_presto.py` keeps it discoverable for future maintainers via `grep`. Tests: parametrize over representative non-`1` values (`0`, `true`, `false`, empty, `banana`) to lock in that none of them skip the daemon.
1 parent a9bc5f5 commit 672c91c

3 files changed

Lines changed: 21 additions & 6 deletions

File tree

ā€Ždocs/source/reference/command_line.rstā€Ž

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,7 @@ Below is a list with all available subcommands.
319319
* Create a default user for the profile (email can be configured through the `--email` option)
320320
* Set up the localhost as a `Computer` and configure it
321321
* Set a number of configuration options with sensible defaults
322-
* Start the daemon (unless `--no-broker` is specified or the
323-
`AIIDA_NO_DAEMON_AUTOSTART` environment variable is set)
322+
* Start the daemon (unless `--no-broker` is specified)
324323
325324
By default the command creates a profile that uses SQLite for the database. For the
326325
message broker, it automatically checks for RabbitMQ running on localhost. If found, it

ā€Žsrc/aiida/cmdline/commands/cmd_presto.pyā€Ž

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,7 @@ def verdi_presto(
184184
* Create a default user for the profile (email can be configured through the `--email` option)
185185
* Set up the localhost as a `Computer` and configure it
186186
* Set a number of configuration options with sensible defaults
187-
* Start the daemon (unless `--no-broker` is specified or the
188-
`AIIDA_NO_DAEMON_AUTOSTART` environment variable is set)
187+
* Start the daemon (unless `--no-broker` is specified)
189188
190189
By default the command creates a profile that uses SQLite for the database. For the message broker, it automatically
191190
checks for RabbitMQ running on localhost. If found, it configures RabbitMQ as the broker. Otherwise, it falls back
@@ -293,7 +292,9 @@ def verdi_presto(
293292

294293
echo.echo_success('Configured the localhost as a computer.')
295294

296-
if broker_backend is not None and not os.environ.get('AIIDA_NO_DAEMON_AUTOSTART'):
295+
# `AIIDA_NO_DAEMON_AUTOSTART` is an internal escape hatch for the Docker init script (see
296+
# `.docker/aiida-core-base/s6-assets/init/aiida-prepare.sh`); deliberately undocumented in `--help`.
297+
if broker_backend is not None and os.environ.get('AIIDA_NO_DAEMON_AUTOSTART') != '1':
297298
from aiida.common.exceptions import ConfigurationError
298299
from aiida.engine.daemon.client import DaemonException, get_daemon_client
299300

ā€Žtests/cmdline/commands/test_presto.pyā€Ž

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,29 @@ def test_presto_starts_daemon(run_cli_command, mock_daemon_client):
118118

119119
@pytest.mark.usefixtures('empty_config')
120120
def test_presto_no_daemon_autostart_env(run_cli_command, mock_daemon_client, monkeypatch):
121-
"""Test that setting ``AIIDA_NO_DAEMON_AUTOSTART`` skips the daemon auto-start."""
121+
"""Test that ``AIIDA_NO_DAEMON_AUTOSTART=1`` skips the daemon auto-start."""
122122
monkeypatch.setenv('AIIDA_NO_DAEMON_AUTOSTART', '1')
123123
result = run_cli_command(verdi_presto, ['--non-interactive'])
124124
assert 'Created new profile' in result.output
125125
assert 'Starting the daemon' not in result.output
126126
mock_daemon_client.start_daemon.assert_not_called()
127127

128128

129+
@pytest.mark.parametrize('value', ('0', 'true', 'false', '', 'banana'))
130+
@pytest.mark.usefixtures('empty_config')
131+
def test_presto_daemon_autostart_env_non_one(run_cli_command, mock_daemon_client, monkeypatch, value):
132+
"""Test that anything other than the literal string ``1`` does not skip the daemon.
133+
134+
Only ``AIIDA_NO_DAEMON_AUTOSTART=1`` is recognized; any other value (including ``0``, ``true``,
135+
typos, or empty string) falls through to "start the daemon" so a misspelled or unrecognized
136+
value doesn't silently disable the daemon.
137+
"""
138+
monkeypatch.setenv('AIIDA_NO_DAEMON_AUTOSTART', value)
139+
result = run_cli_command(verdi_presto, ['--non-interactive'])
140+
assert 'Starting the daemon' in result.output
141+
mock_daemon_client.start_daemon.assert_called_once_with()
142+
143+
129144
@pytest.mark.usefixtures('empty_config')
130145
def test_presto_no_broker_skips_daemon(run_cli_command, mock_daemon_client):
131146
"""Test that ``verdi presto --no-broker`` skips the daemon auto-start (no broker means no daemon)."""

0 commit comments

Comments
Ā (0)
⚔