crate / crash

Crash is an interactive CrateDB command line interface (CLI) SQL shell with autocompletion.
Apache License 2.0
48 stars 13 forks source link

better catch unauthorized errors with forced password prompt #424

Closed proddata closed 8 months ago

proddata commented 8 months ago

About

Better catch unauthorized errors with forced password prompt instead of displaying stack trace, when using the -W option.

 % crash --hosts 'https://elite-krait.eks1.eu-west-1.aws.cratedb.net:4200' -U 'admin' -W
Password: 
401 Client Error: Unauthorized

Before, it was like:

``` Password: Traceback (most recent call last): File "/path/to/.venv/bin/crash", line 8, in sys.exit(main()) ^^^^^^ File "/path/to/.venv/lib/python3.11/site-packages/crate/crash/command.py", line 589, in main raise e File "/path/to/.venv/lib/python3.11/site-packages/crate/crash/command.py", line 575, in main cmd = _create_shell(crate_hosts, error_trace, output_writer, is_tty, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/.venv/lib/python3.11/site-packages/crate/crash/command.py", line 622, in _create_shell return CrateShell(crate_hosts, ^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/.venv/lib/python3.11/site-packages/crate/crash/command.py", line 228, in __init__ self._connect(crate_hosts) File "/path/to/.venv/lib/python3.11/site-packages/crate/crash/command.py", line 323, in _connect self.connection = connect(servers, ^^^^^^^^^^^^^^^^ File "/path/to/.venv/lib/python3.11/site-packages/crate/client/connection.py", line 152, in __init__ self.lowest_server_version = self._lowest_server_version() ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/.venv/lib/python3.11/site-packages/crate/client/connection.py", line 196, in _lowest_server_version _, _, version = self.client.server_infos(server) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/.venv/lib/python3.11/site-packages/crate/client/http.py", line 461, in server_infos _raise_for_status(response) File "/path/to/.venv/lib/python3.11/site-packages/crate/client/http.py", line 199, in _raise_for_status return _raise_for_status_real(response) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/path/to/.venv/lib/python3.11/site-packages/crate/client/http.py", line 231, in _raise_for_status_real raise ProgrammingError(message) crate.client.exceptions.ProgrammingError: 401 Client Error: Unauthorized ```

References

amotl commented 8 months ago

Thanks for the patch.

I agree to unify the behavior between -W and <n/a> when prompting for passwords, unless there are other objections. There may have been specific reasons for the current behaviour to discriminate between tty vs. non-interactive environments, but personally I can't see any, as long as is_tty is respected properly.

To check/ask back for that, I am also adding @seut and @mfussenegger as reviewers.