Adding Config and db query Support #71
Conversation
Introduced YAML configuration file handling using `click-config-file` and `pyyaml`, ensuring a default config file is created if not present. Updated dependencies in `pyproject.toml` accordingly to support the new feature.
Introduced a new `configure` command group in the CLI for managing configurations. Implemented `set` and `get` commands to update and retrieve configuration values, respectively. Added necessary unit tests for these functionalities.
WalkthroughThe changes enhance a command-line interface (CLI) application by introducing commands for configuration and database management. New functions facilitate the creation and verification of configuration files, while structured command groups enable users to set, get, and display configuration values. Additionally, utilities for managing database connections and executing SQL queries streamline interactions through the command line. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ConfigUtils
participant Configure
participant DB
User->>CLI: Run config command
CLI->>ConfigUtils: ensure_config_file()
ConfigUtils-->>CLI: Configuration file ready
CLI->>Configure: Process command (set/get/show)
Configure->>ConfigUtils: set_config_value()/get_config_value()/list_config()
ConfigUtils-->>Configure: Confirm action
Configure-->>CLI: Display result
CLI-->>User: Show output
User->>CLI: Run db query command
CLI->>DB: Execute query()
DB-->>CLI: Return results
CLI-->>User: Show query results
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Outside diff range, codebase verification and nitpick comments (1)
src/hckr/cli/configure.py (1)
56-78: Thegetcommand is well-implemented but consider handling more specific exceptions.The command retrieves a configuration value and handles
ValueError. Consider handling more specific exceptions if applicable.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pyproject.tomlis excluded by!**/*.toml
Files selected for processing (4)
- src/hckr/cli/init.py (3 hunks)
- src/hckr/cli/configure.py (1 hunks)
- src/hckr/utils/ConfigUtils.py (1 hunks)
- tests/cli/test_configure.py (1 hunks)
Additional context used
Ruff
tests/cli/test_configure.py
2-2:
distutils.command.config.configimported but unusedRemove unused import:
distutils.command.config.config(F401)
src/hckr/cli/configure.py
2-2:
loggingimported but unusedRemove unused import:
logging(F401)
6-6:
cron_descriptor.get_descriptionimported but unusedRemove unused import:
cron_descriptor.get_description(F401)
9-9:
..utils.MessageUtilsimported but unusedRemove unused import:
..utils.MessageUtils(F401)
10-10:
..utils.ConfigUtils.load_configimported but unusedRemove unused import
(F401)
10-10:
..utils.ConfigUtils.config_pathimported but unusedRemove unused import
(F401)
src/hckr/cli/__init__.py
5-5:
osimported but unused(F401)
src/hckr/utils/ConfigUtils.py
5-5:
clickimported but unusedRemove unused import:
click(F401)
Additional comments not posted (13)
tests/cli/test_configure.py (1)
9-12: Utility function_get_random_stringlooks good.The function generates a random string of a specified length using lowercase letters.
src/hckr/cli/configure.py (3)
13-23: Theconfigurecommand group is well-defined.The command group sets up configuration-related commands and ensures the configuration file exists.
30-54: Thesetcommand is well-implemented.The command sets a configuration value and provides user feedback using
rich.
81-86: Theshowcommand is well-implemented.The command lists configuration values using
ConfigUtils.src/hckr/cli/__init__.py (3)
54-68: Theensure_config_filefunction is well-implemented.The function ensures the configuration file and its directory exist, and writes a default configuration if necessary.
73-77: Theyaml_loaderfunction is well-implemented.The function loads a YAML configuration file and retrieves data based on the command.
Line range hint
83-98: CLI setup and command registration are well-implemented.The CLI is set up with configuration options and commands are registered correctly.
src/hckr/utils/ConfigUtils.py (6)
17-21: LGTM!The
load_configfunction correctly loads the configuration file usingConfigParser.
24-51: LGTM!The
ensure_config_filefunction correctly ensures the configuration file's existence and writes a default configuration if necessary.
54-79: LGTM!The
set_config_valuefunction correctly sets a configuration value and handles section creation if necessary.
82-95: LGTM!The
get_config_valuefunction correctly retrieves a configuration value, ensuring the section and key exist.
98-118: LGTM!The
list_configfunction correctly lists configuration values for a specified section usingrich.
121-125: LGTM!The
configMessagefunction correctly logs the configuration being used.
tests/cli/test_configure.py
Outdated
| def test_configure_get_set_default(): | ||
| runner = CliRunner() | ||
| _key = f"key_{_get_random_string(5)}" | ||
| _value = f"value_{_get_random_string(5)}" | ||
| result = runner.invoke(set, [_key, _value]) | ||
| assert result.exit_code == 0 | ||
| print(result.output) | ||
|
|
||
| assert ( | ||
| f"Set [DEFAULT] {_key} = {_value}" | ||
| in result.output | ||
| ) | ||
| result = runner.invoke(get, [_key, _value]) |
There was a problem hiding this comment.
Ensure the get command is tested correctly.
The test_configure_get_set_default function tests the set command but does not verify the get command output.
Consider modifying the test to check if the get command retrieves the correct value:
result = runner.invoke(get, [_key])
assert result.exit_code == 0
assert (
f"[DEFAULT] {_key} = {_value}"
in result.output
)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_configure_get_set_default(): | |
| runner = CliRunner() | |
| _key = f"key_{_get_random_string(5)}" | |
| _value = f"value_{_get_random_string(5)}" | |
| result = runner.invoke(set, [_key, _value]) | |
| assert result.exit_code == 0 | |
| print(result.output) | |
| assert ( | |
| f"Set [DEFAULT] {_key} = {_value}" | |
| in result.output | |
| ) | |
| result = runner.invoke(get, [_key, _value]) | |
| def test_configure_get_set_default(): | |
| runner = CliRunner() | |
| _key = f"key_{_get_random_string(5)}" | |
| _value = f"value_{_get_random_string(5)}" | |
| result = runner.invoke(set, [_key, _value]) | |
| assert result.exit_code == 0 | |
| print(result.output) | |
| assert ( | |
| f"Set [DEFAULT] {_key} = {_value}" | |
| in result.output | |
| ) | |
| result = runner.invoke(get, [_key]) | |
| assert result.exit_code == 0 | |
| assert ( | |
| f"[DEFAULT] {_key} = {_value}" | |
| in result.output | |
| ) |
src/hckr/cli/configure.py
Outdated
|
|
||
| import click | ||
| import rich | ||
| from cron_descriptor import get_description # type: ignore |
There was a problem hiding this comment.
Remove unused import.
The get_description import from cron_descriptor is not used in this file.
Apply this diff to remove the unused import:
-from cron_descriptor import get_description # type: ignoreCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from cron_descriptor import get_description # type: ignore |
Tools
Ruff
6-6:
cron_descriptor.get_descriptionimported but unusedRemove unused import:
cron_descriptor.get_description(F401)
Remove redundant configurations and modularize configuration handling. Add a new CLI command to configure database credentials interactively, enhancing user flexibility.
Introduced `DbUtils` for handling database URLs based on configuration and added database command functionalities. Updated existing configurations to integrate with the new database utility functions and commands. Enhanced test coverage and consistency in config handling.
Introduce a new 'config' command group for managing configuration values. This includes implementing commands to set, get, and show configuration values. Additionally, enhanced the database configuration options to include database type selection and more detailed prompts.
Standardize usage of double quotes in DbUtils.py for consistency. Fix missing imports and improve SQLite handling in config. Adjust pyproject.toml to replace psycopg2 with psycopg2-binary and add snowflake-sqlalchemy.
Introduced a new `init` command to create a configuration file if it doesn't exist. Refactored the configuration tests to separate default and custom configurations and added missing key/value test cases. Adjusted config handling to ensure the presence of a config file before setting up databases.
Introduced a new `init` command to create a configuration file if it doesn't exist. Refactored the configuration tests to separate default and custom configurations and added missing key/value test cases. Adjusted config handling to ensure the presence of a config file before setting up databases.
Refactored database configuration handling to support multiple DB types and improve error messaging. Enhanced tests to validate configuration logic and renamed test files/methods for better clarity. Added logging and updated CLI command imports for improved code organization.
Refactor the `query` command to execute SQL on Snowflake and return results as a DataFrame. Added imports for `config_path`, `print_df_as_table`, and `pandas`. Replaced row-by-row output with a formatted DataFrame table display.
Added parameters to control the number of rows and columns displayed in `print_df_as_table` function. Updated function calls to use these new parameters, allowing for more flexible and customized table prints.
Enhanced the `query` command to allow specifying the number of rows and columns displayed in the output with `--num-rows` and `--num-cols` options. The default values for these options are set to 10.
There was a problem hiding this comment.
Actionable comments posted: 18
Outside diff range, codebase verification and nitpick comments (2)
src/hckr/utils/DataUtils.py (1)
Line range hint
24-45: Enhanceprint_df_as_tablewith configurable column count.The function now allows specifying the number of columns to display, which enhances its flexibility.
The changes are well-implemented and improve the function's usability. However, address the unused loop variable for cleaner code.
Rename the unused loop variable to underscore:
-for index, row in df.head(count).iterrows(): +for _index, row in df.head(count).iterrows():Tools
Ruff
43-43: Loop control variable
indexnot used within loop bodyRename unused
indexto_index(B007)
tests/cli/test_config.py (1)
72-77: Nitpick: Consistency in error message assertions.While the test for missing value correctly asserts the presence of an error message, it could be improved by ensuring consistency in how error messages are checked across all tests.
Consider using a consistent method for asserting error messages to improve the maintainability of the test suite.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pyproject.tomlis excluded by!**/*.toml
Files selected for processing (10)
- src/hckr/cli/init.py (2 hunks)
- src/hckr/cli/config.py (1 hunks)
- src/hckr/cli/configure.py (1 hunks)
- src/hckr/cli/db.py (1 hunks)
- src/hckr/utils/ConfigUtils.py (1 hunks)
- src/hckr/utils/DataUtils.py (2 hunks)
- src/hckr/utils/DbUtils.py (1 hunks)
- src/hckr/utils/FileUtils.py (3 hunks)
- src/hckr/utils/MessageUtils.py (2 hunks)
- tests/cli/test_config.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/hckr/cli/init.py
Additional context used
Ruff
src/hckr/cli/db.py
4-4:
yaspin.yaspinimported but unusedRemove unused import:
yaspin.yaspin(F401)
7-7:
hckr.utils.ConfigUtils.config_pathimported but unusedRemove unused import:
hckr.utils.ConfigUtils.config_path(F401)
src/hckr/utils/DbUtils.py
2-2:
configparser.NoSectionErrorimported but unusedRemove unused import:
configparser.NoSectionError(F401)
4-4:
sqlalchemy.create_engineimported but unusedRemove unused import
(F401)
4-4:
sqlalchemy.textimported but unusedRemove unused import
(F401)
5-5:
sqlalchemy.exc.SQLAlchemyErrorimported but unusedRemove unused import:
sqlalchemy.exc.SQLAlchemyError(F401)
6-6:
clickimported but unusedRemove unused import:
click(F401)
9-9:
hckr.utils.ConfigUtils.load_configimported but unusedRemove unused import:
hckr.utils.ConfigUtils.load_config(F401)
src/hckr/cli/config.py
5-5:
cron_descriptor.get_descriptionimported but unusedRemove unused import:
cron_descriptor.get_description(F401)
src/hckr/utils/DataUtils.py
43-43: Loop control variable
indexnot used within loop bodyRename unused
indexto_index(B007)
src/hckr/cli/configure.py
3-3:
..utils.MessageUtilsimported but unusedRemove unused import:
..utils.MessageUtils(F401)
105-105: Undefined name
account(F821)
src/hckr/utils/ConfigUtils.py
5-5:
readline.set_completerimported but unusedRemove unused import:
readline.set_completer(F401)
7-7:
clickimported but unusedRemove unused import:
click(F401)
11-11:
.MessageUtils.PMsgimported but unusedRemove unused import
(F401)
11-11:
.MessageUtils.PErrorimported but unusedRemove unused import
(F401)
84-84: f-string without any placeholders
Remove extraneous
fprefix(F541)
162-162: f-string without any placeholders
Remove extraneous
fprefix(F541)
Additional comments not posted (8)
src/hckr/cli/db.py (1)
13-18: Command group setup is correct.The
dbfunction correctly sets up a command group for database operations.src/hckr/utils/FileUtils.py (2)
22-22: Type annotation added for clarity.The addition of the
-> FileFormatreturn type annotation infileExtToFormatmethod clarifies the expected return type, aligning with Python best practices for type hinting.
125-125: Type annotation added for clarity.The addition of the
-> FileFormatreturn type annotation inget_file_format_from_extensionfunction clarifies the expected return type, enhancing type safety and reducing potential runtime errors.src/hckr/utils/ConfigUtils.py (1)
7-7: Remove unused import.The import
clickis not used in this file.Apply this diff to remove the unused import:
-import clickLikely invalid or redundant comment.
Tools
Ruff
7-7:
clickimported but unusedRemove unused import:
click(F401)
src/hckr/utils/MessageUtils.py (1)
45-46: Approved: Specialized message functions.The specialized functions
PWarn,PSuccess,PInfo, andPErroreffectively abstract the common pattern of usingPMsgwith different titles. This enhances code readability and usability.Also applies to: 49-50, 53-54, 57-58
tests/cli/test_config.py (3)
16-28: Approved: Comprehensive tests for config get and set.The tests for getting and setting configuration values are well-structured and cover the basic functionality expected from the CLI. Using random strings for keys and values is a good practice to ensure test isolation.
30-43: Approved: Tests for custom configuration.The tests for handling custom configurations are thorough and validate the functionality as expected. This ensures that the CLI can handle multiple configuration contexts effectively.
45-55: Approved: Configuration display test.The test for the
showcommand effectively checks the output for both default and all configurations. This is crucial for ensuring that the CLI provides accurate feedback to the user.
src/hckr/utils/DbUtils.py
Outdated
| def _get_snowflake_url(config, section): | ||
| user = config.get(section, "user") | ||
| password = config.get(section, "password") | ||
| account = config.get(section, "account") | ||
| warehouse = config.get(section, "warehouse") | ||
| role = config.get(section, "role") | ||
| dbname = config.get(section, "dbname") | ||
| schema = config.get(section, 'schema') | ||
| return ( | ||
| f"snowflake://{user}:{password}@{account}/{dbname}/{schema}" | ||
| f"?warehouse={warehouse}&role={role}" | ||
| ) |
There was a problem hiding this comment.
Validate parameters in _get_snowflake_url.
The function _get_snowflake_url constructs a URL for Snowflake databases but does not validate the parameters before using them.
Add parameter validation to ensure all required fields are present and valid before constructing the URL.
Add checks for parameter existence and validity:
if not all([user, password, account, warehouse, role, dbname, schema]):
raise ValueError("Missing required configuration parameters for Snowflake URL")
src/hckr/utils/ConfigUtils.py
Outdated
| "config_type": "default", | ||
| }, | ||
| "CUSTOM": { | ||
| "key": f"value", |
There was a problem hiding this comment.
Remove extraneous f prefix from strings.
The f string prefix is used without placeholders, which is unnecessary.
Apply this diff to correct the strings:
-"CUSTOM": {
- "key": f"value",
+"CUSTOM": {
+ "key": "value",-title=f"\[DEFAULT]",
+title="\[DEFAULT]",Also applies to: 162-162
Tools
Ruff
84-84: f-string without any placeholders
Remove extraneous
fprefix(F541)
src/hckr/utils/ConfigUtils.py
Outdated
| import logging | ||
| from enum import Enum | ||
| from pathlib import Path | ||
| from readline import set_completer |
There was a problem hiding this comment.
Remove unused import.
The import set_completer from readline is not used in this file.
Apply this diff to remove the unused import:
-from readline import set_completerCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from readline import set_completer |
Tools
Ruff
5-5:
readline.set_completerimported but unusedRemove unused import:
readline.set_completer(F401)
src/hckr/utils/ConfigUtils.py
Outdated
| import rich | ||
| from rich.panel import Panel | ||
| from . import MessageUtils | ||
| from .MessageUtils import PWarn, PMsg, PError, PSuccess, PInfo |
There was a problem hiding this comment.
Remove unused imports.
The imports PMsg and PError from MessageUtils are not used in this file.
Apply this diff to remove the unused imports:
-from .MessageUtils import PWarn, PMsg, PError, PSuccess, PInfo
+from .MessageUtils import PWarn, PSuccess, PInfoCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from .MessageUtils import PWarn, PMsg, PError, PSuccess, PInfo | |
| from .MessageUtils import PWarn, PSuccess, PInfo |
Tools
Ruff
11-11:
.MessageUtils.PMsgimported but unusedRemove unused import
(F401)
11-11:
.MessageUtils.PErrorimported but unusedRemove unused import
(F401)
src/hckr/utils/MessageUtils.py
Outdated
| def PMsg(msg, title): | ||
| rich.print( | ||
| Panel( | ||
| msg, | ||
| expand=False, | ||
| title=title, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Refactor suggestion: Consider using keyword arguments for clarity.
The PMsg function uses positional arguments which might lead to confusion or errors in usage. Consider refactoring to use keyword arguments for better clarity and maintainability.
-def PMsg(msg, title):
+def PMsg(msg, title=""):Committable suggestion was skipped due to low confidence.
Enable logging message uncommented in MessageUtils and revised set_config_value to support override flag. Cleaned up redundant configuration tests and added an informational placeholder test. Refactor message utilities and update CLI config commands Refactored `PMsg` to `_PMsg` for internal use and added optional descriptions. Updated CLI config commands to leverage new utilities for success and error messaging. Enhanced configuration setting to include informative messages when adding new sections. Update version to 0.4.0.dev0 This updates the version from 0.3.3.dev0 to 0.4.0.dev0 in the __about__.py file. This change likely indicates new features or significant improvements. Refactor code formatting for improved readability This commit updates the code formatting by changing single quotes to double quotes in SQL queries and aligns parameters across multiple functions and fixtures. Additionally, it ensures better transaction handling in database operations by using context management to automatically commit DDL and non-data-returning DML queries. These changes enhance code readability and maintainability without altering functionality. Add CLI tests for DB queries and refactor query handling Introduced new tests in `test_db.py` to cover various database query scenarios. Refactored the `query` function in `db.py` to better handle data and non-data-returning queries. Configuration utility functions for Add configuration utilities and database command refactoring Introduced new configuration utilities for managing database settings. Updated the `DbUtils` module to use centralized constants for configuration options, and refactored `configure_db` command for improved readability and maintenance. Added comprehensive tests for various database options. Refactor config utilities and enhance config file verification Renamed config utilities path for better organization and clarity. Enhanced `config_exists` function to check if the config file is not only present but also non-empty. Improved messages and functionality in various utility methods, and slightly adjusted message display in `MessageUtils.py`.
There was a problem hiding this comment.
Actionable comments posted: 14
Outside diff range, codebase verification and nitpick comments (1)
tests/cli/test_db.py (1)
7-11: Remove unnecessary print statement.The
Apply this diff to remove the
- print(result.output)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- .gitignore (1 hunks)
- src/hckr/about.py (1 hunks)
- src/hckr/cli/config.py (1 hunks)
- src/hckr/cli/configure.py (1 hunks)
- src/hckr/cli/db.py (1 hunks)
- src/hckr/utils/DbUtils.py (1 hunks)
- src/hckr/utils/FileUtils.py (1 hunks)
- src/hckr/utils/MessageUtils.py (2 hunks)
- src/hckr/utils/config/ConfigUtils.py (1 hunks)
- src/hckr/utils/config/ConfigureUtils.py (1 hunks)
- src/hckr/utils/config/Constants.py (1 hunks)
- tests/cli/test_cli.py (1 hunks)
- tests/cli/test_configure.py (1 hunks)
- tests/cli/test_db.py (1 hunks)
- tests/conftest.py (1 hunks)
Files skipped from review due to trivial changes (3)
- .gitignore
- src/hckr/about.py
- src/hckr/utils/FileUtils.py
Files skipped from review as they are similar to previous changes (2)
- src/hckr/utils/DbUtils.py
- src/hckr/utils/MessageUtils.py
Additional context used
Ruff
tests/cli/test_configure.py
2-2:
hckr.utils.MessageUtils._PMsgimported but unusedRemove unused import:
hckr.utils.MessageUtils._PMsg(F401)
src/hckr/utils/config/ConfigureUtils.py
1-1:
configparser.NoOptionErrorimported but unusedRemove unused import:
configparser.NoOptionError(F401)
7-7:
.Constants.ConfigTypeimported but unusedRemove unused import
(F401)
9-9:
.Constants.DB_NAMEimported but unusedRemove unused import
(F401)
11-11:
.Constants.CONFIG_TYPEimported but unusedRemove unused import
(F401)
12-12:
.Constants.DB_TYPEimported but unusedRemove unused import
(F401)
21-21:
..DbUtils._get_jdbc_urlimported but unusedRemove unused import
(F401)
21-21:
..DbUtils._get_snowflake_urlimported but unusedRemove unused import
(F401)
22-22:
..MessageUtils.PErrorimported but unusedRemove unused import:
..MessageUtils.PError(F401)
src/hckr/cli/config.py
4-4:
richimported but unusedRemove unused import:
rich(F401)
5-5:
cron_descriptor.get_descriptionimported but unusedRemove unused import:
cron_descriptor.get_description(F401)
src/hckr/cli/db.py
5-5:
yaspin.yaspinimported but unusedRemove unused import:
yaspin.yaspin(F401)
10-10:
hckr.utils.MessageUtils.PSuccessimported but unusedRemove unused import:
hckr.utils.MessageUtils.PSuccess(F401)
src/hckr/cli/configure.py
6-6:
..utils.config.ConfigUtils.DBTypeimported but unusedRemove unused import:
..utils.config.ConfigUtils.DBType(F401)
15-15:
..utils.config.Constants.DB_HOSTimported but unusedRemove unused import
(F401)
16-16:
..utils.config.Constants.DB_PORTimported but unusedRemove unused import
(F401)
17-17:
..utils.config.Constants.DB_ACCOUNTimported but unusedRemove unused import
(F401)
18-18:
..utils.config.Constants.DB_WAREHOUSEimported but unusedRemove unused import
(F401)
19-19:
..utils.config.Constants.DB_SCHEMAimported but unusedRemove unused import
(F401)
20-20:
..utils.config.Constants.DB_ROLEimported but unusedRemove unused import
(F401)
21-21:
..utils.config.Constants.DB_PASSWORDimported but unusedRemove unused import
(F401)
23-23:
..utils.config.Constants.DB_USERimported but unusedRemove unused import
(F401)
src/hckr/utils/config/ConfigUtils.py
8-8:
..MessageUtils.PErrorimported but unusedRemove unused import:
..MessageUtils.PError(F401)
10-10:
.Constants.DBTypeimported but unusedRemove unused import:
.Constants.DBType(F401)
70-70: f-string without any placeholders
Remove extraneous
fprefix(F541)
149-149: f-string without any placeholders
Remove extraneous
fprefix(F541)
Additional comments not posted (29)
tests/cli/test_cli.py (1)
21-31: LGTM!The function correctly tests the CLI commands and includes the new commands in the help output.
src/hckr/utils/config/Constants.py (4)
1-6: LGTM!The imports and constants are correctly defined.
8-15: LGTM!The
DBTypeenum is correctly defined and the__str__method is correctly implemented.
18-22: LGTM!The
ConfigTypeenum is correctly defined and the__str__method is correctly implemented.
25-49: LGTM!The
db_type_mappingdictionary and configuration constants are correctly defined.tests/cli/test_configure.py (4)
5-11: LGTM!The function is correctly implemented and tests the configuration of a PostgreSQL database.
13-19: LGTM!The function is correctly implemented and tests the configuration of a MySQL database.
21-27: LGTM!The function is correctly implemented and tests the configuration of a Snowflake database.
29-35: LGTM!The function is correctly implemented and tests the configuration of an SQLite database.
tests/cli/test_db.py (3)
14-39: LGTM!The code changes are approved.
45-50: LGTM!The code changes are approved.
53-58: LGTM!The code changes are approved.
src/hckr/utils/config/ConfigureUtils.py (2)
25-49: LGTM!The code changes are approved.
52-64: LGTM!The code changes are approved.
tests/conftest.py (6)
16-17: LGTM!The code changes are approved.
21-23: LGTM!The code changes are approved.
27-43: LGTM!The code changes are approved.
47-63: LGTM!The code changes are approved.
67-87: LGTM!The code changes are approved.
91-99: LGTM!The code changes are approved.
src/hckr/cli/config.py (3)
74-86: LGTM!The code changes are approved.
88-102: LGTM!The code changes are approved.
18-27: Expand the docstring for theconfigcommand group.The docstring for the
configcommand group should provide more information about the commands included in this group.Expand the docstring as follows:
""" -Defines a command group for configuration-related commands. +Defines a command group for managing application configurations. This group includes commands to set, get, show, and initialize configuration values. """Likely invalid or redundant comment.
src/hckr/cli/configure.py (1)
27-36: LGTM!The code changes are approved.
src/hckr/utils/config/ConfigUtils.py (5)
30-45: LGTM!The code changes are approved.
92-118: LGTM!The code changes are approved.
121-134: LGTM!The code changes are approved.
176-193: LGTM!The code changes are approved.
196-200: LGTM!The code changes are approved.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/hckr/cli/config.py (1 hunks)
Additional context used
Ruff
src/hckr/cli/config.py
4-4:
richimported but unusedRemove unused import:
rich(F401)
5-5:
cron_descriptor.get_descriptionimported but unusedRemove unused import:
cron_descriptor.get_description(F401)
Additional comments not posted (5)
src/hckr/cli/config.py (5)
88-102: LGTM!The code changes are approved.
18-27: Document the purpose ofconfigcommand group more clearly.The
configcommand group is defined without much detail in the docstring.Enhance the docstring to provide more information about the commands included in this group.
Expand the docstring:
""" -Defines a command group for configuration-related commands. +Defines a command group for managing application configurations. This group includes commands to set, get, show, and initialize configuration values. """Likely invalid or redundant comment.
4-5: Remove unused imports.The imports
richandcron_descriptor.get_descriptionare not used in this file and should be removed to clean up the code.Apply this diff to remove the unused imports:
-import rich -import cron_descriptor.get_description # type: ignoreLikely invalid or redundant comment.
Tools
Ruff
4-4:
richimported but unusedRemove unused import:
rich(F401)
5-5:
cron_descriptor.get_descriptionimported but unusedRemove unused import:
cron_descriptor.get_description(F401)
61-72: Improve error handling ingetcommand.The
getcommand retrieves a configuration value but lacks comprehensive error handling for missing or invalid keys.Enhance error handling to provide more specific feedback to the user:
try: value = get_config_value(config, key) except ValueError as e: - PError(f"{e}") + rich.print(Panel(f"Configuration key not found: {key}", title="Error", expand=True))Likely invalid or redundant comment.
40-59: Ensure clarity and correctness insetcommand documentation and functionality.The
setcommand sets a configuration value but the documentation could be clearer and the function might benefit from additional error handling.Improve the docstring and add error handling:
""" Sets a configuration value. + + This command updates the configuration for the specified key with the given value. If the key does not exist, it will be created. Args: config (str): The configuration instance name. Default is defined by DEFAULT_CONFIG. key (str): The key of the config setting to change. value (str): The value to set for the specified key. + + Raises: + ValueError: If the key is invalid or the value is not acceptable for the key. """ +try: + set_config_value(config, key, value) +except ValueError as e: + rich.print(Panel(str(e), title="Error", expand=True)) + returnLikely invalid or redundant comment.
Removed `exit(1)` calls from DbUtils.py to allow for better exception handling and application flow. Enhanced documentation for config commands in config.py for clearer guidance on usage and examples.
Refactored SQL query handling for better readability and maintainability by streamlining conditional checks and removing redundant checks. Additionally, cleaned up various utility files by removing unnecessary imports and docstring comments.
The commented-out import statement was removed to clean up the code. This change helps maintain readability and ensures there are no unnecessary lines in the file.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- src/hckr/cli/config.py (1 hunks)
- src/hckr/cli/configure.py (1 hunks)
- src/hckr/cli/db.py (1 hunks)
- src/hckr/utils/DbUtils.py (1 hunks)
- src/hckr/utils/config/ConfigUtils.py (1 hunks)
- src/hckr/utils/config/ConfigureUtils.py (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- src/hckr/cli/config.py
- src/hckr/cli/configure.py
- src/hckr/utils/DbUtils.py
- src/hckr/utils/config/ConfigureUtils.py
Additional context used
Ruff
src/hckr/utils/config/ConfigUtils.py
51-51: f-string without any placeholders
Remove extraneous
fprefix(F541)
Additional comments not posted (9)
src/hckr/cli/db.py (3)
20-78: Enhance error handling and robustness in thequerycommand.The function generally handles the SQL query execution well. However, it could improve by ensuring the database URL is retrieved before attempting to connect.
Consider checking the
db_urlimmediately after retrieval and handling the case where it might beNonemore robustly:db_url = get_db_url(section=config) query = query.strip() -if db_url: +if not db_url: + PError("Database credentials are not properly configured.") + return if db_url: engine = create_engine(db_url) try: with engine.connect() as connection: # Normalize and determine the type of query normalized_query = query.lower() is_data_returning_query = normalized_query.startswith( ("select", "desc", "describe", "show", "explain") ) is_ddl_query = normalized_query.startswith( ("create", "alter", "drop", "truncate") ) if is_data_returning_query: # Execute and fetch results for queries that return data df = pd.read_sql_query(text(query), connection) # Optionally limit rows and columns if specified if num_rows is not None: df = df.head(num_rows) if num_cols is not None: df = df.iloc[:, :num_cols] print_df_as_table(df, title=query) return df else: # Execute DDL or non-data-returning DML queries with connection.begin(): # this will automatically commit at the end result = connection.execute(text(query)) if is_ddl_query: PInfo(query, "Success") else: PInfo(query, f"[Success] Rows affected: {result.rowcount}") except SQLAlchemyError as e: PError(f"Error executing query: {e}") else: PError("Database credentials are not properly configured.")
1-9: Remove unused imports.The imports
config_pathfromhckr.utils.ConfigUtils,yaspin, andPSuccessare not used within this file and should be removed to clean up the code.Apply this diff to remove the unused imports:
-from hckr.utils.ConfigUtils import config_path -from yaspin import yaspin # type: ignore -from hckr.utils.MessageUtils import PSuccessLikely invalid or redundant comment.
12-17: Add a docstring for thedbcommand group.The
dbcommand group should have a docstring to explain its purpose.Add a docstring as follows:
@click.group( help="Database commands", context_settings={"help_option_names": ["-h", "--help"]}, ) def db(): + """ + Defines a command group for database-related commands. + """ passLikely invalid or redundant comment.
src/hckr/utils/config/ConfigUtils.py (6)
26-38: LGTM!The code changes are approved.
41-71: LGTM!The code changes are approved.
Tools
Ruff
51-51: f-string without any placeholders
Remove extraneous
fprefix(F541)
73-85: LGTM!The code changes are approved.
88-95: LGTM!The code changes are approved.
98-134: Remove unnecessaryfprefix.The
fprefix in the f-string is unnecessary and should be removed.Apply this diff to remove the unnecessary
fprefix:title=f"\[DEFAULT]", + title="\[DEFAULT]",Likely invalid or redundant comment.
1-10: Remove unused imports.The following imports are unused and should be removed:
..MessageUtils.PError.Constants.DBTypeApply this diff to remove the unused imports:
-from ..MessageUtils import PWarn, PSuccess, PInfo, PError +from ..MessageUtils import PWarn, PSuccess, PInfo -from .Constants import config_path, DBType, DEFAULT_CONFIG +from .Constants import config_path, DEFAULT_CONFIGLikely invalid or redundant comment.
Introduced detailed Sphinx documentation for config and configure commands. Enhanced CLI help strings with examples and notes to guide users through the configuration process, including database setup.
Updated the documentation to include a new section for database commands, with specific instructions and examples on how to configure and use these commands. Enhanced the `query` function docstring to provide detailed usage examples, including options for limiting rows and columns.
github-actions
bot
added
the
documentation
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- docs/source/commands/config/config.rst (1 hunks)
- docs/source/commands/config/configure.rst (1 hunks)
- docs/source/commands/config/index.rst (1 hunks)
- docs/source/commands/database/db.rst (1 hunks)
- docs/source/commands/database/index.rst (1 hunks)
- docs/source/commands/index.rst (2 hunks)
- src/hckr/cli/config.py (1 hunks)
- src/hckr/cli/configure.py (1 hunks)
- src/hckr/cli/db.py (1 hunks)
Files skipped from review due to trivial changes (5)
- docs/source/commands/config/config.rst
- docs/source/commands/config/configure.rst
- docs/source/commands/config/index.rst
- docs/source/commands/database/db.rst
- docs/source/commands/database/index.rst
Files skipped from review as they are similar to previous changes (2)
- src/hckr/cli/configure.py
- src/hckr/cli/db.py
Additional comments not posted (8)
docs/source/commands/index.rst (2)
8-13: LGTM!The addition of the "Configure" section improves the navigational structure of the documentation.
37-42: LGTM!The addition of the "Database" section improves the navigational structure of the documentation.
src/hckr/cli/config.py (6)
1-11: LGTM!The imports are necessary for the functionality of the file.
14-23: LGTM!The command group is correctly defined and the docstring provides sufficient information.
26-33: LGTM!The decorator is correctly implemented and adds a useful option for configuration instance.
36-63: LGTM!The command is correctly implemented and the docstring is comprehensive.
66-97: Improve error handling in thegetcommand.The
getcommand retrieves a configuration value but lacks comprehensive error handling for missing or invalid keys.Enhance error handling to provide more specific feedback to the user.
Refine the error handling mechanism:
try: value = get_config_value(config, key) except ValueError as e: - PError(f"{e}") + PError(f"Configuration key not found: {key}")
100-170: LGTM!Both commands are correctly implemented and the docstrings are comprehensive.
Updated all CLI test functions to accept the cli_runner fixture instead of creating new CliRunner instances. This ensures better test consistency and readability. All occurrences of 'runner' were replaced with 'cli_runner' to reflect this change.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/cli/test_config.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/cli/test_config.py
Added `src/hckr/__about__.py` and all `__init__.py` files to exclusion list to prevent them from being considered main source files in SonarQube analysis. This helps in focusing the static analysis on relevant pieces of code.
|
closes #23 #69