✨ Auto-start daemon in verdi presto#7351
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7351 +/- ##
==========================================
+ Coverage 80.27% 80.29% +0.03%
==========================================
Files 577 577
Lines 45530 45543 +13
==========================================
+ Hits 36544 36564 +20
+ Misses 8986 8979 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
agoscinski
added a commit
to agoscinski/aiida-core
that referenced
this pull request
Apr 29, 2026
When a broker is configured, `verdi presto` now automatically starts the daemon after profile setup. If the daemon fails to start, a warning is shown without failing the command.
agoscinski
force-pushed
the
feat/presto-daemon-autostart
branch
from
9a7f318 to
58cf75a
Compare
agoscinski
added a commit
to agoscinski/aiida-core
that referenced
this pull request
Apr 29, 2026
When a broker is configured, `verdi presto` now automatically starts the daemon after profile setup. If the daemon fails to start, a warning is shown without failing the command.
agoscinski
force-pushed
the
feat/presto-daemon-autostart
branch
from
58cf75a to
478b194
Compare
agoscinski
added a commit
to agoscinski/aiida-core
that referenced
this pull request
Apr 30, 2026
When a broker is configured, `verdi presto` now automatically starts the daemon after profile setup. If the daemon fails to start, a warning is shown without failing the command.
agoscinski
force-pushed
the
feat/presto-daemon-autostart
branch
from
478b194 to
5ebbda0
Compare
agoscinski
added a commit
to agoscinski/aiida-core
that referenced
this pull request
Apr 30, 2026
When a broker is configured, `verdi presto` now automatically starts the daemon after profile setup. If the daemon fails to start, a warning is shown without failing the command. Replace `verdi presto` with `verdi profile setup core.psql_dos` in the Docker container's `aiida-prepare.sh` because `verdi presto` auto-starting the daemon conflicts with the s6-overlay startup sequence (the editable install in `run-before-daemon-start` must complete before the daemon starts).
agoscinski
force-pushed
the
feat/presto-daemon-autostart
branch
from
5ebbda0 to
d252d46
Compare
agoscinski
added a commit
to agoscinski/aiida-core
that referenced
this pull request
Apr 30, 2026
When a broker is configured, `verdi presto` now automatically starts the daemon after profile setup. If the daemon fails to start, a warning is shown without failing the command. Add `--no-daemon-autostart` flag to skip the daemon auto-start. This is needed in the Docker container where s6-overlay manages the daemon lifecycle separately (the editable install in `run-before-daemon-start` must complete before the daemon starts).
agoscinski
force-pushed
the
feat/presto-daemon-autostart
branch
from
d252d46 to
5ed5cdd
Compare
Member
|
One alternative: instead of an option, which is user facing and pollutes the API, can we read an env variable instead? So inside a Docker container one could run: |
agoscinski
added a commit
to agoscinski/aiida-core
that referenced
this pull request
Apr 30, 2026
When a broker is configured, `verdi presto` now automatically starts the daemon after profile setup. If the daemon fails to start, a warning is shown without failing the command. Add `--no-daemon-autostart` flag to skip the daemon auto-start. This is needed in the Docker container where s6-overlay manages the daemon lifecycle separately (the editable install in `run-before-daemon-start` must complete before the daemon starts).
agoscinski
force-pushed
the
feat/presto-daemon-autostart
branch
from
5ed5cdd to
04a3890
Compare
agoscinski
added a commit
to agoscinski/aiida-core
that referenced
this pull request
Apr 30, 2026
When a broker is configured, `verdi presto` now automatically starts the daemon after profile setup. If the daemon fails to start, a warning is shown without failing the command. Add `--no-daemon-autostart` flag to skip the daemon auto-start. This is needed in the Docker container where s6-overlay manages the daemon lifecycle separately (the editable install in `run-before-daemon-start` must complete before the daemon starts).
agoscinski
force-pushed
the
feat/presto-daemon-autostart
branch
from
04a3890 to
61455d2
Compare
Collaborator
Author
Yes true could work. It's mainly for setting up aiida in CI environments where setting env vars is not annoying. Maybe |
Collaborator
Agree with this! Adding a commit rn. |
When a broker is configured, `verdi presto` now automatically starts the daemon after profile setup. If the daemon fails to start, a warning is shown without failing the command. Add `--no-daemon-autostart` flag to skip the daemon auto-start. This is needed in the Docker container where s6-overlay manages the daemon lifecycle separately (the editable install in `run-before-daemon-start` must complete before the daemon starts).
The daemon auto-start in `verdi presto` previously caught the broad `Exception`, which would silently swallow unrelated programmer errors (`AttributeError`, `TypeError`, etc.) under a friendly "Failed to start daemon" warning and hide regressions in nearby code paths. Catch only `DaemonException` (the documented failure mode of `DaemonClient.start_daemon`) and `ConfigurationError` (raised by `get_daemon_client` for an unknown profile). Anything else now propagates as a real traceback. Tests: - Rewrite `test_presto_daemon_start_failure` to inject a `DaemonException` through the shared `mock_daemon_client` fixture instead of redefining its own monkeypatch and raising a `RuntimeError` (which only worked under the broad except). Also assert the "verdi daemon start" follow-up hint is shown. - Tighten `test_presto_starts_daemon` from `assert_called_once()` to `assert_called_once_with()` so any future change to the call signature is caught. - Add `test_presto_no_broker_skips_daemon` covering the `--no-broker` branch of the skip guard. - Move `mock_daemon_client` into `@pytest.mark.usefixtures` for tests that only need the side effect, and drop an unused `pytestconfig` parameter from `test_presto_without_rmq`.
Replace `--no-daemon-autostart` with the environment variable `AIIDA_NO_DAEMON_AUTOSTART`. The flag exists to serve scripted container/CI setups, not interactive users; an env var keeps that contract out of the user-facing CLI surface and matches the existing `AIIDA_*` prefix convention (`AIIDA_PATH`, `AIIDA_BROKER_*`, `AIIDA_TEST_PROFILE`, etc.). Suggested by @mbercx and refined by @agoscinski in PR review. Update `aiida-prepare.sh` accordingly: prepend `AIIDA_NO_DAEMON_AUTOSTART=1` to the `verdi presto` invocation instead of passing the flag. Tests: replace `test_presto_no_daemon_autostart` with `test_presto_no_daemon_autostart_env`, exercising the env var via `monkeypatch.setenv`.
GeigerJ2
force-pushed
the
feat/presto-daemon-autostart
branch
from
d608c59 to
a9bc5f5
Compare
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.
Collaborator
|
Added myself as co-author, bc I iterated a bit on the PR, and @mbercx, as he brought up the env var idea; and having ideas is the actual real work nowadays, not the prompting to get the code :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I would have preferred to not add an option
--no-daemon-autostartbut the docker image needs to be flexible for different environments and automatically detect and initialize the aiida profile according to them which would have resulted in copying the auto-configuration fromverdi prestointo the CI.When a broker is configured,
verdi prestonow automatically starts the daemon after profile setup. If the daemon fails to start, a warning is shown without failing the command.Add
--no-daemon-autostartflag to skip the daemon auto-start. This is needed in the Docker container where s6-overlay manages the daemon lifecycle separately (the editable install inrun-before-daemon-startmust complete before the daemon starts).