You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2023/01/05 12:07:17 UTC

[GitHub] [incubator-kvrocks] torwig opened a new pull request, #1217: Config load errors

torwig opened a new pull request, #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217

   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
   ```
   
   2. 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 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1217: Config load errors

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217#discussion_r1063058509


##########
utils/kvrocks2redis/kvrocks2redis.conf:
##########
@@ -23,7 +29,7 @@ kvrocks 127.0.0.1 6666
 
 # Enable cluster mode.
 #
-# Default: yes
+# Default: no
 cluster-enable yes

Review Comment:
   Is there a typo?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] torwig merged pull request #1217: Make config load errors more friendly

Posted by GitBox <gi...@apache.org>.
torwig merged PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1217: Make config load errors more friendly

Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217#discussion_r1063138982


##########
utils/kvrocks2redis/kvrocks2redis.conf:
##########
@@ -23,7 +29,7 @@ kvrocks 127.0.0.1 6666
 
 # Enable cluster mode.
 #
-# Default: yes
+# Default: no
 cluster-enable yes

Review Comment:
   Yes, the default value should be `no`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] torwig commented on pull request #1217: Make config load errors more friendly

Posted by GitBox <gi...@apache.org>.
torwig commented on PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217#issuecomment-1373256854

   Thanks all. Merging...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1217: Make config load errors more friendly

Posted by GitBox <gi...@apache.org>.
torwig commented on code in PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217#discussion_r1063135350


##########
utils/kvrocks2redis/kvrocks2redis.conf:
##########
@@ -23,7 +29,7 @@ kvrocks 127.0.0.1 6666
 
 # Enable cluster mode.
 #
-# Default: yes
+# Default: no
 cluster-enable yes

Review Comment:
   @PragmaTwice You mean the default is `no` but the actual value in the config file is `yes`? I was thinking about the change. Should I change it to `no`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1217: Make config load errors more friendly

Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217#issuecomment-1373179714

   > 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, it should have the slot prefix if the cluster mode was enabled.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1217: Make config load errors more friendly

Posted by GitBox <gi...@apache.org>.
torwig commented on code in PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217#discussion_r1063141889


##########
utils/kvrocks2redis/kvrocks2redis.conf:
##########
@@ -23,7 +29,7 @@ kvrocks 127.0.0.1 6666
 
 # Enable cluster mode.
 #
-# Default: yes
+# Default: no
 cluster-enable yes

Review Comment:
   Changed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1217: Make config load errors more friendly

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217#discussion_r1063136824


##########
utils/kvrocks2redis/kvrocks2redis.conf:
##########
@@ -23,7 +29,7 @@ kvrocks 127.0.0.1 6666
 
 # Enable cluster mode.
 #
-# Default: yes
+# Default: no
 cluster-enable yes

Review Comment:
   Yeah but I am not familiar with these components. cc @git-hulk 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1217: Make config load errors more friendly

Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217#discussion_r1063136962


##########
utils/kvrocks2redis/kvrocks2redis.conf:
##########
@@ -23,7 +29,7 @@ kvrocks 127.0.0.1 6666
 
 # Enable cluster mode.
 #
-# Default: yes
+# Default: no
 cluster-enable yes

Review Comment:
   This configuration should be meanless to kvrocks2redis



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] torwig commented on pull request #1217: Make config load errors more friendly

Posted by GitBox <gi...@apache.org>.
torwig commented on PR #1217:
URL: https://github.com/apache/incubator-kvrocks/pull/1217#issuecomment-1373178433

   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).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org