Vamsi-klu commented on PR #63579: URL: https://github.com/apache/airflow/pull/63579#issuecomment-4061375840
Hey @choo121600, good find — accidentally approving workflow runs by pressing a random key is definitely a safety concern for the triage tool. The `while True` loop approach is the right fix and it matches how `prompt_triage_action` in the same file already handles this. One thing to watch out for: there's a **merge conflict** with current main. Commit 7cea0b83ac landed a refactor that replaced `get_console().print()` with `console_print()` across the codebase. You'll need to rebase and update those calls. A couple of other thoughts: - **Tests would help here.** There are no existing tests for `user_confirm`, and since the behavioral change is significant (re-prompt instead of silent default), even a couple of targeted tests would go a long way. For example, mocking `_read_char` to return "x" then "y" and asserting `Answer.YES` would prove unrecognized keys trigger a re-prompt. Same for multi-byte escape sequences. - **Reviewer @bugraoz93 suggested a flag approach** instead of `while True` — I think `while True` is the right call here since it's idiomatic Python for this pattern and matches the existing code in the same file. Might be worth responding to that comment to close the loop. The logic itself looks correct to me across all the edge cases I checked (Enter with/without default, case sensitivity, quit_allowed=False, special characters). Clean and well-scoped fix. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
