You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "torwig (via GitHub)" <gi...@apache.org> on 2023/06/24 13:27:16 UTC

[GitHub] [kvrocks] torwig commented on a diff in pull request #1518: convert timeout parameter from int to double

torwig commented on code in PR #1518:
URL: https://github.com/apache/kvrocks/pull/1518#discussion_r1240779435


##########
src/commands/cmd_list.cc:
##########
@@ -284,7 +286,7 @@ class CommandBPop : public Commander,
 
  private:
   bool left_ = false;
-  int timeout_ = 0;  // seconds
+  int64_t timeout_ = 0;  // microsecond

Review Comment:
   `microsecond` => `microseconds` with `s` and in similar places.



##########
src/commands/cmd_list.cc:
##########
@@ -165,16 +165,16 @@ class CommandBPop : public Commander,
   ~CommandBPop() override = default;
 
   Status Parse(const std::vector<std::string> &args) override {
-    auto parse_result = ParseInt<int>(args[args.size() - 1], 10);
+    auto parse_result = ParseFloat(args[args.size() - 1]);
     if (!parse_result) {
-      return {Status::RedisParseErr, "timeout is not an integer or out of range"};
+      return {Status::RedisParseErr, errTimeoutIsNotFloat};
     }
 
     if (*parse_result < 0) {
       return {Status::RedisParseErr, "timeout should not be negative"};
     }
 
-    timeout_ = *parse_result;
+    timeout_ = static_cast<int64_t>(*parse_result * 1000 * 1000);  // microsecond

Review Comment:
   I think here you can remove the comment. In same places as well.



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