Re: [PR] feat(cmd): add blocking flag and remove useless flags [kvrocks]

2024-11-02 Thread via GitHub


sonarcloud[bot] commented on PR #2637:
URL: https://github.com/apache/kvrocks/pull/2637#issuecomment-2453048426

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2637) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [2 New 
issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=2637&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=2637&issueStatuses=ACCEPTED)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=2637&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [61.0% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2637&metric=new_coverage&view=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [1.1% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2637&metric=new_duplicated_lines_density&view=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2637)
   
   


-- 
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-unsubscr...@kvrocks.apache.org

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



Re: [PR] feat(cmd): add blocking flag and remove useless flags [kvrocks]

2024-11-02 Thread via GitHub


PragmaTwice merged PR #2637:
URL: https://github.com/apache/kvrocks/pull/2637


-- 
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-unsubscr...@kvrocks.apache.org

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



Re: [PR] feat(cmd): add blocking flag and remove useless flags [kvrocks]

2024-11-02 Thread via GitHub


sonarcloud[bot] commented on PR #2637:
URL: https://github.com/apache/kvrocks/pull/2637#issuecomment-2452958220

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2637) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [2 New 
issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=2637&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=2637&issueStatuses=ACCEPTED)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=2637&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [61.0% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2637&metric=new_coverage&view=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [1.1% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=2637&metric=new_duplicated_lines_density&view=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2637)
   
   


-- 
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-unsubscr...@kvrocks.apache.org

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



Re: [PR] feat(cmd): add blocking flag and remove useless flags [kvrocks]

2024-11-02 Thread via GitHub


git-hulk commented on code in PR #2637:
URL: https://github.com/apache/kvrocks/pull/2637#discussion_r1826527147


##
src/server/redis_connection.cc:
##
@@ -425,8 +425,11 @@ void Connection::ExecuteCommands(std::deque 
*to_process_cmds) {
   concurrency = srv_->WorkConcurrencyGuard();
 }
 
-if (cmd_flags & kCmdROScript) {
-  // if executing read only lua script commands, set current connection.
+auto category = attributes->category;
+if ((category == CommandCategory::Function || category == 
CommandCategory::Script) && (cmd_flags & kCmdReadOnly)) {
+  // FIXME: since read-only script commands are not exclusive,
+  // SetCurrentConnection here is weird and can cause many issues,
+  // we should pass the Connection direcly to the lua context instead

Review Comment:
   ```suggestion
 // we should pass the Connection directly to the lua context instead
   ```



-- 
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-unsubscr...@kvrocks.apache.org

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