🎨 Palette: Add validation to interactive prompts#127
Conversation
- Added `get_valid_input` helper for robust interactive prompts - Integrated validation for Profile ID and API Token in `main()` - Refactored `validate_profile_id` to support silent validation - Added tests for new validation logic This prevents long wait times before failure due to simple typos in interactive mode.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Merging to
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| monkeypatch.setattr(m, "log", mock_log) | ||
|
|
||
| # Invalid ID with logging enabled (default) | ||
| assert m.validate_profile_id("invalid spaces") is False |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| # Invalid ID with logging enabled (default) | ||
| assert m.validate_profile_id("invalid spaces") is False | ||
| assert mock_log.error.called |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| mock_log.reset_mock() | ||
|
|
||
| # Invalid ID with logging disabled | ||
| assert m.validate_profile_id("invalid spaces", log_errors=False) is False |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| # Invalid ID with logging disabled | ||
| assert m.validate_profile_id("invalid spaces", log_errors=False) is False | ||
| assert not mock_log.error.called |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| result = m.get_valid_input("Prompt: ", validator, "Error message") | ||
|
|
||
| assert result == "valid" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| result = m.get_valid_input("Prompt: ", validator, "Error message") | ||
|
|
||
| assert result == "valid" | ||
| assert input_mock.call_count == 3 |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| # Check output for error messages | ||
| captured = capsys.readouterr() | ||
| assert "Value cannot be empty" in captured.out |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Check output for error messages | ||
| captured = capsys.readouterr() | ||
| assert "Value cannot be empty" in captured.out | ||
| assert "Error message" in captured.out |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| result = m.get_valid_input("Password: ", validator, "Error", is_password=True) | ||
|
|
||
| assert result == "secret" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| return False | ||
| return True | ||
|
|
||
| def get_valid_input(prompt: str, validator: callable, error_msg: str, is_password: bool = False) -> str: |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Line too long (104/100) Warning
| f"{Colors.BOLD}Enter Control D API Token:{Colors.ENDC} ", | ||
| validate_token, | ||
| "Token seems too short. Please check your API token.", | ||
| is_password=True |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
More than one statement on a single line Warning
| assert input_mock.call_count == 3 | ||
|
|
||
| # Check output for error messages | ||
| captured = capsys.readouterr() |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Missing function or method docstring Warning test
| result = m.get_valid_input("Password: ", validator, "Error", is_password=True) | ||
|
|
||
| assert result == "secret" | ||
| getpass_mock.assert_called_once() |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Missing function or method docstring Warning test
test_main.py
Outdated
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Missing function or method docstring Warning test
| return True | ||
|
|
||
| def validate_profile_id(profile_id: str) -> bool: | ||
| def validate_profile_id(profile_id: str, log_errors: bool = True) -> bool: |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning
| return False | ||
| return True | ||
|
|
||
| def get_valid_input(prompt: str, validator: callable, error_msg: str, is_password: bool = False) -> str: |
Check warning
Code scanning / Pylint (reported by Codacy)
Line too long (104/100) Warning
|
|
||
| t_input = get_valid_input( | ||
| f"{Colors.BOLD}Enter Control D API Token:{Colors.ENDC} ", | ||
| validate_token, |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning
| f"{Colors.BOLD}Enter Control D API Token:{Colors.ENDC} ", | ||
| validate_token, | ||
| "Token seems too short. Please check your API token.", | ||
| is_password=True |
Check warning
Code scanning / Pylint (reported by Codacy)
More than one statement on a single line Warning
main.py
Outdated
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning
| assert input_mock.call_count == 3 | ||
|
|
||
| # Check output for error messages | ||
| captured = capsys.readouterr() |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning test
| result = m.get_valid_input("Password: ", validator, "Error", is_password=True) | ||
|
|
||
| assert result == "secret" | ||
| getpass_mock.assert_called_once() |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning test
test_main.py
Outdated
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning test
| if is_password: | ||
| value = getpass.getpass(prompt).strip() | ||
| else: | ||
| value = input(prompt).strip() |
Check notice
Code scanning / Bandit (reported by Codacy)
The input method in Python 2 will read from standard input, evaluate and run the resulting string as python source code. This is similar, though in many ways worse, then using eval. On Python 2, use raw_input instead, input is safe in Python 3. Note
|
|
||
| # Case 10: get_valid_input works with getpass | ||
| def test_get_valid_input_password(monkeypatch): | ||
| m = reload_main_with_env(monkeypatch) |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| monkeypatch.setattr('getpass.getpass', getpass_mock) | ||
|
|
||
| validator = lambda x: True | ||
|
|
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| validator = lambda x: True | ||
|
|
||
| result = m.get_valid_input("Password: ", validator, "Error", is_password=True) |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
test_main.py
Outdated
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
test_main.py
Outdated
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
test_main.py
Outdated
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
test_main.py
Outdated
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
test_main.py
Outdated
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
There was a problem hiding this comment.
Pull request overview
This PR adds immediate validation for interactive CLI prompts to catch invalid profile IDs and tokens before any long-running operations, improving UX and error clarity.
Changes:
- Extended
validate_profile_idwith alog_errorsflag to support silent validation in interactive flows while preserving existing logging behavior for other callers. - Introduced a
get_valid_inputhelper that wrapsinput()/getpass.getpass()in a validation loop with clear, colored error messages and integrated it into the profile ID and token prompts inmain(). - Added unit tests for the new behavior and documented the pattern in
.Jules/palette.mdunder “Immediate CLI Input Validation”.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
main.py |
Adds log_errors to validate_profile_id, defines get_valid_input, and rewires interactive profile ID and token prompts to use the new helper for immediate validation. |
test_main.py |
Extends test coverage to verify validate_profile_id’s logging toggle and the new get_valid_input behavior for both normal and password-style input. |
.Jules/palette.md |
Documents the new interactive validation pattern and codifies get_valid_input as the preferred wrapper for prompting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return False | ||
| return True | ||
|
|
||
| def get_valid_input(prompt: str, validator: callable, error_msg: str, is_password: bool = False) -> str: |
There was a problem hiding this comment.
The type annotation for validator uses the built-in callable, which isn't a proper typing hint and won't be understood correctly by static type checkers; prefer using typing.Callable[[str], bool] (or a suitable Callable signature) for clearer intent and better tooling support.
🎨 Palette: Add validation to interactive prompts
💡 What: Added immediate input validation to the CLI interactive prompts.
🎯 Why: To prevent users from waiting for cache warm-up (potentially long) only to fail due to a typo in the Profile ID or Token.
♿ Accessibility: Provides clear, immediate error messages ("❌ Value cannot be empty") instead of generic failures.
PR created automatically by Jules for task 9923405677569176900 started by @abhimehro