You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2022/09/09 07:42:59 UTC
[GitHub] [zeppelin] huage1994 opened a new pull request, #4462: [MINOR] [JDBC] JDBCInterpreterTest.testMultiTenant_* improvement
huage1994 opened a new pull request, #4462:
URL: https://github.com/apache/zeppelin/pull/4462
### What is this PR for?
Now In test cases `JDBCInterpreterTest.testMultiTenant_1` and `JDBCInterpreterTest.testMultiTenant_1`, the jdbc interpreter would try to connect h2 databases with 3 non-existed user.
1. It would cost more time to execute these two test case. The execution time of every test case time would be reduced from over 1 second to 50 milliseconds if the users is exsited.
2. And also the interpreter would log a long message about `JdbcSQLInvalidAuthorizationSpecException` , which may mislead developer who is unfamiliar to JDBC module into thinking there is something wrong with the test case。
```
03:41:28.204 [main] ERROR org.apache.zeppelin.jdbc.JDBCInterpreter - Fail to getConnection
org.h2.jdbc.JdbcSQLInvalidAuthorizationSpecException: Wrong user name or password [28000-206]
at org.h2.message.DbException.getJdbcSQLException(DbException.java:529) ~[h2-2.0.206.jar:2.0.206]
at org.h2.message.DbException.getJdbcSQLException(DbException.java:496) ~[h2-2.0.206.jar:2.0.206]
at org.h2.message.DbException.get(DbException.java:227) ~[h2-2.0.206.jar:2.0.206]
at org.h2.message.DbException.get(DbException.java:203) ~[h2-2.0.206.jar:2.0.206]
at org.h2.message.DbException.get(DbException.java:192) ~[h2-2.0.206.jar:2.0.206]
at org.h2.engine.Engine.validateUserAndPassword(Engine.java:393) ~[h2-2.0.206.jar:2.0.206]
at org.h2.engine.Engine.createSession(Engine.java:206) ~[h2-2.0.206.jar:2.0.206]
at org.h2.engine.SessionRemote.connectEmbeddedOrServer(SessionRemote.java:338) ~[h2-2.0.206.jar:2.0.206]
at org.h2.jdbc.JdbcConnection.<init>(JdbcConnection.java:117) ~[h2-2.0.206.jar:2.0.206]
at org.h2.Driver.connect(Driver.java:59) ~[h2-2.0.206.jar:2.0.206]
at java.sql.DriverManager.getConnection(DriverManager.java:664) ~[?:1.8.0_345]
at java.sql.DriverManager.getConnection(DriverManager.java:208) ~[?:1.8.0_345]
```
This PR to resolve this problem by the way to create these 3 users in `setup()`.
### What type of PR is it?
Improvement
### Todos
* [ ] - Task
### What is the Jira issue?
* Open an issue on Jira [ZEPPELIN-5816](https://issues.apache.org/jira/browse/ZEPPELIN-5816)
### How should this be tested?
CI passed.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? 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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] huage1994 commented on pull request #4462: [ZEPPELIN-5816] [MINOR] [JDBC] JDBCInterpreterTest.testMultiTenant_* improvement
Posted by GitBox <gi...@apache.org>.
huage1994 commented on PR #4462:
URL: https://github.com/apache/zeppelin/pull/4462#issuecomment-1246176952
> So if I understand the unit tests correctly, the following asserts only test the properties and not the successful connection. Correct?
>
> https://github.com/apache/zeppelin/blob/bbe46d737d6738c0a6cc2b58de90bbb16c6b3c55/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java#L571-L573
Yes. The asserts and this test case run well without any error.
It is the following line of code to try to connect h2, and print error-level log about `JdbcSQLInvalidAuthorizationSpecException`. Actually It does not affect the running of the entire test case. This PR is only to remove the confusing and useless error-level log about `JdbcSQLInvalidAuthorizationSpecException` .
https://github.com/apache/zeppelin/blob/23f3acf2086d78630dbc3d18016cb7e40c390ef2/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java#L579
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4462: [ZEPPELIN-5816] [MINOR] [JDBC] JDBCInterpreterTest.testMultiTenant_* improvement
Posted by GitBox <gi...@apache.org>.
Reamer commented on PR #4462:
URL: https://github.com/apache/zeppelin/pull/4462#issuecomment-1243314576
Did you find out why the tests run successfully even without users?
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4462: [ZEPPELIN-5816] [MINOR] [JDBC] JDBCInterpreterTest.testMultiTenant_* improvement
Posted by GitBox <gi...@apache.org>.
Reamer commented on PR #4462:
URL: https://github.com/apache/zeppelin/pull/4462#issuecomment-1245507867
So if I understand the unit tests correctly, the following asserts only test the properties and not the successful connection. Correct?
https://github.com/apache/zeppelin/blob/bbe46d737d6738c0a6cc2b58de90bbb16c6b3c55/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java#L571-L573
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] huage1994 commented on pull request #4462: [ZEPPELIN-5816] [MINOR] [JDBC] JDBCInterpreterTest.testMultiTenant_* improvement
Posted by GitBox <gi...@apache.org>.
huage1994 commented on PR #4462:
URL: https://github.com/apache/zeppelin/pull/4462#issuecomment-1244832988
> Did you find out why the tests run successfully even without users?
😄 Sure.
By default there is a user named '' with the password '' in the h2 database with admin privilege.
The user can be show by executing `select * from INFORMATION_SCHEMA.USERS` .
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer merged pull request #4462: [ZEPPELIN-5816] [MINOR] [JDBC] JDBCInterpreterTest.testMultiTenant_* improvement
Posted by GitBox <gi...@apache.org>.
Reamer merged PR #4462:
URL: https://github.com/apache/zeppelin/pull/4462
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org