You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "kevinjmh (via GitHub)" <gi...@apache.org> on 2024/03/14 03:06:37 UTC
[PR] [ZEPPELIN-6003] Log detail info of SQL in JDBCInterpreter [zeppelin]
kevinjmh opened a new pull request, #4732:
URL: https://github.com/apache/zeppelin/pull/4732
### What is this PR for?
Currently we can monitor running SQL in JDBCInterpreter but unable to know who/which note fire the SQL, especially a couple of users using zeppelin at the same time.
Bond the SQL with additional info, we can inform the user with user/noteid/paragragh info if needed
### What type of PR is it?
Improvement
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-6003
### How should this be tested?
* Strongly recommended: add automated unit tests for any new or changed behavior
* Outline any manual steps to test the PR here.
### Screenshots (if appropriate)
### Questions:
* Does the license files need to update?
* Is there breaking changes for older versions?
* Does this needs documentation?
--
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: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6003] Log source info of SQL in JDBCInterpreter [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on code in PR #4732:
URL: https://github.com/apache/zeppelin/pull/4732#discussion_r1524546853
##########
jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java:
##########
@@ -799,7 +800,8 @@ private InterpreterResult executeSql(String sql,
// so we need to trim it first in this case.
sqlToExecute = sqlToExecute.trim();
}
- LOGGER.info("Execute sql: " + sqlToExecute);
+ LOGGER.info(String.format("[%s|%s|%s] Execute sql: %s",
+ user, noteId, paragraphId, sqlToExecute));
Review Comment:
Yes, I also think it's better.
--
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: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6003] Log source info of SQL in JDBCInterpreter [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4732:
URL: https://github.com/apache/zeppelin/pull/4732#discussion_r1524498089
##########
jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java:
##########
@@ -799,7 +800,8 @@ private InterpreterResult executeSql(String sql,
// so we need to trim it first in this case.
sqlToExecute = sqlToExecute.trim();
}
- LOGGER.info("Execute sql: " + sqlToExecute);
+ LOGGER.info(String.format("[%s|%s|%s] Execute sql: %s",
+ user, noteId, paragraphId, sqlToExecute));
Review Comment:
```suggestion
LOGGER.info("[{}|{}|{}] Execute sql: {}", user, noteId, paragraphId, sqlToExecute);
```
See: https://rules.sonarsource.com/java/RSPEC-2629/
and https://www.slf4j.org/api/org/slf4j/Logger.html#info(java.lang.String,java.lang.Object...)
--
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: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6003] Log source info of SQL in JDBCInterpreter [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer merged PR #4732:
URL: https://github.com/apache/zeppelin/pull/4732
--
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: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6003] Log source info of SQL in JDBCInterpreter [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4732:
URL: https://github.com/apache/zeppelin/pull/4732#discussion_r1524498089
##########
jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java:
##########
@@ -799,7 +800,8 @@ private InterpreterResult executeSql(String sql,
// so we need to trim it first in this case.
sqlToExecute = sqlToExecute.trim();
}
- LOGGER.info("Execute sql: " + sqlToExecute);
+ LOGGER.info(String.format("[%s|%s|%s] Execute sql: %s",
+ user, noteId, paragraphId, sqlToExecute));
Review Comment:
```suggestion
LOGGER.info("[{}|{}|{}] Execute sql: {}", user, noteId, paragraphId, sqlToExecute);
```
See: https://rules.sonarsource.com/java/RSPEC-2629/
--
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: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org