Skip to content
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

Make config load errors more friendly #1217

Merged
merged 3 commits into from
Jan 6, 2023

Conversation

torwig
Copy link
Contributor

@torwig torwig commented Jan 5, 2023

Enrich error messages that can occur during the loading of the configuration for both Kvrocks and Kvrocks2Redis.
Minor refactoring of edited source files.
Add a description of default values to Kvrocks2Redis configuration file (cluster-enable changed to no because in the source code, the default value is false).

Examples of new error messages

  1. Kvrocks
Failed to load config. Error: failed to open file '/home/yaroslav/tests/kvrocks/kvrocks3.conf': No such file or directory
Failed to load config. Error: at line #L52: failed to set value of field 'cluster-enabled': argument must be 'yes' or 'no'
Failed to load config. Error: at line #L332: compaction-checker-range is invalid: invalid range format, the range should be between 0 and 24
Failed to load config. Error: at line #L24: malformed config line: config line ends unexpectedly in quoted string
  1. Kvrocks2Redis
Failed to load config. Error: the specified Kvrocks working directory './kvrocks-data/' doesn't exist
Failed to load config. Error: at line #L22: Kvrocks port value should be between 0 and 65535
Failed to load config. Error: failed to open file '/home/yaroslav/tests/kvrocks/kvrocks2redis2.conf': No such file or directory

Closes: #637

git-hulk
git-hulk previously approved these changes Jan 5, 2023
@PragmaTwice PragmaTwice changed the title Config load errors Make config load errors more friendly Jan 6, 2023
@torwig
Copy link
Contributor Author

torwig commented Jan 6, 2023

It seems like the cluster-enable config option isn't used across the kvrocks2redis codebase. But I'll double-check it later and do the refactoring if it's true (in another PR).

@git-hulk
Copy link
Member

git-hulk commented Jan 6, 2023

It seems like the cluster-enable config option isn't used across the kvrocks2redis codebase. But I'll double-check it later and do the refactoring if it's true (in another PR).

It's used when decoding the key, coz the key should have the slot prefix if the cluster mode was enabled.

@torwig
Copy link
Contributor Author

torwig commented Jan 6, 2023

Thanks all. Merging...

@torwig torwig merged commit 4ddee2f into apache:unstable Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: More user-friendly error message
3 participants