-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix history file usage in SQL CLI tool. #1077
Fix history file usage in SQL CLI tool. #1077
Conversation
Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>
Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>
Fix history file usage in SQL CLI tool.
Codecov Report
@@ Coverage Diff @@
## 2.x #1077 +/- ##
============================================
- Coverage 98.27% 95.71% -2.57%
Complexity 3351 3351
============================================
Files 327 337 +10
Lines 8457 9115 +658
Branches 553 672 +119
============================================
+ Hits 8311 8724 +413
- Misses 142 334 +192
- Partials 4 57 +53
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -57,9 +61,21 @@ def __init__(self, clirc_file=None, always_use_pager=False, use_aws_authenticati | |||
self.multiline_continuation_char = config["main"]["multiline_continuation_char"] | |||
self.multi_line = config["main"].as_bool("multi_line") | |||
self.multiline_mode = config["main"].get("multi_line_mode", "src") | |||
self.history_file = config["main"]["history_file"] | |||
self.log_file = config["main"]["log_file"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this being used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
History file used for history, log file - for logging errors. For example, if config has incorrect path to the history file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it - do you need to pass this in anywhere like you do the history file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No actually. Good point, thank you. self.log_file
removed in f79e5a9.
Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Signed-off-by: Yury-Fridlyand yuryf@bitquilltech.com
Description
Changes include:
FileHistory
.history_file
andlog_file
parameters from config.Config location:
~/.config/opensearchsql-cli/config
Default location for saving history and log files:
~/.config/opensearchsql-cli
Inspired by: https://github.com/prompt-toolkit/python-prompt-toolkit/blob/master/examples/prompts/history/persistent-history.py
See team review at Bit-Quill#155
Test
venv
(see instructions below)How to test
Ref: https://github.com/opensearch-project/sql/blob/2.x/sql-cli/development_guide.md#development-environment-set-up
Do once:
Then every time you want to debug SQL CLI:
To exit from
venv
:You can run SQL CLI and switch branch. It will keep working with given timeout until closed.
Issues Resolved
Query history didn't persist across SQL CLI runs.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.