Conversation
….py, add typing_extensions dep Address Paul's review comment: fix remaining Self imports that used 'Any as Self' fallback on Python 3.10, and add typing_extensions as an explicit dependency in pyproject.toml.
typer.confirm raises click.Abort when stdin is not a TTY. New dimos/utils/prompt.py provides confirm() that returns the default when not interactive. system_configurator now uses it. Revert: git revert HEAD
dimos/utils/prompt.py provides: - confirm(): returns default if not TTY, else typer.confirm() - sudo_run(): prepends sudo if not root Moved sudo_run from system_configurator/base.py to prompt.py. Updated all importers (base.py, lcm.py, clock_sync.py, tests). Root cause: Ivan's 8edc995 replaced input() with typer.confirm() which raises click.Abort on non-TTY stdin. Revert: git revert HEAD
| return False | ||
|
|
||
|
|
||
| def sudo_run(*args: Any, **kwargs: Any) -> subprocess.CompletedProcess[str]: |
There was a problem hiding this comment.
moved to prompt.py (because sudo often needs stdin)
|
|
||
| def confirm(message: str, *, default: bool = True) -> bool: | ||
| """Ask yes/no""" | ||
| if not sys.stdin.isatty(): |
Greptile SummaryThis PR restores the non-interactive fix for Key changes:
Items to consider:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["configure_system()"] --> B{"CI env set?"}
B -- Yes --> Z["return (skip)"]
B -- No --> C["run checks"]
C --> D{"any failing?"}
D -- No --> Z
D -- Yes --> E["prompt.confirm()"]
E --> F{"sys.stdin.isatty()?"}
F -- No --> G["return default=True"]
F -- Yes --> H["typer.confirm(message, default=True)"]
G --> I{"confirmed?"}
H --> I
I -- No --> J{"any critical?"}
J -- Yes --> K["raise SystemExit(1)"]
J -- No --> Z
I -- Yes --> L["for each failing check: check.fix()"]
L --> M["prompt.sudo_run(cmd)"]
M --> N{"os.geteuid() == 0?"}
N -- Yes --> O["subprocess.run(cmd)"]
N -- No --> P["subprocess.run(['sudo', ...cmd])"]
Reviews (1): Last reviewed commit: "fix: add prompt.confirm() and prompt.sud..." | Re-trigger Greptile |
The test_lcm_autodecoder_pubsub test flaked under full suite load because publish() fired before the handler thread entered the LCM loop. Adding a 50ms sleep after start() in each fixture gives the thread time to be ready.
mustafab0
left a comment
There was a problem hiding this comment.
Looks pretty good to me.
PR should be ready to merge once the reptile comments are resolved
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Non-interactive (no tty) still defaults to True for daemons/CI. Interactive sessions no longer default to yes — pressing Enter without typing y/n now aborts instead of silently applying system changes. Prevents accidental sudo commands from Enter-mashing.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
I am almost certain we already fixed this (if non-interactive, assume yes for the autoconf prompts). But I don't see the fix anymore.
Problem
After a reboot, when trying to run tests in a non-interactive shell, the ask about networking config breaks and causes tests to fail.
Solution
There's a more general problem, which is asking for prompts/sudo after a module has already started (I wanted to do this for docker pull). A minimal
prompts.pyutil was created which should allow for easy shimming later for that case.Breaking Changes
None
How to Test
reboot
Contributor License Agreement