Skip to content

Conversation

@astroshim
Copy link
Contributor

@astroshim astroshim commented Sep 2, 2016

What is this PR for?

This PR is for refactoring code for JDBCInterpreter.
There is no putting 'Connection' to 'propertyKeyUnusedConnectionListMap' anywhere in the original code.

What type of PR is it?

Improvement

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1405

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? no

@astroshim
Copy link
Contributor Author

\cc @jongyoul Please review this PR if i misunderstand.

@jongyoul
Copy link
Member

jongyoul commented Sep 3, 2016

@astroshim It doesn't look completed. When are some connections closed? First of all, there's no code for closing connections.

@astroshim
Copy link
Contributor Author

astroshim commented Sep 3, 2016

@jongyoul I will refactor the code to use connection pool.

@jongyoul
Copy link
Member

jongyoul commented Sep 3, 2016

In this PR or separately?

On Saturday, 3 September 2016, HyungSung [email protected] wrote:

@jongyoul https://github.com/jongyoul I will refactor the code to use
database pool.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1396 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADcflgJmeDQxwU2fRTGSZURNtH5EZb6Yks5qmQ5ngaJpZM4Jzgoo
.

이종열, Jongyoul Lee, 李宗烈
http://madeng.net

@astroshim
Copy link
Contributor Author

I will do it in this PR and change the title this. :)

@jongyoul
Copy link
Member

jongyoul commented Sep 3, 2016

Good!!

On Saturday, 3 September 2016, HyungSung [email protected] wrote:

I will do it in this PR and change the title this. :)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1396 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADcflk02U-1wmCZXO3P3cKSDLcjpISYAks5qmROygaJpZM4Jzgoo
.

이종열, Jongyoul Lee, 李宗烈
http://madeng.net

@astroshim astroshim changed the title [ZEPPELIN-1405] Remove propertyKeyUnusedConnectionListMap variable in JDBCInterpreter [ZEPPELIN-1405] ConnectionPool to JDBCInterpreter. Sep 4, 2016
@astroshim astroshim changed the title [ZEPPELIN-1405] ConnectionPool to JDBCInterpreter. [ZEPPELIN-1405] ConnectionPool for JDBCInterpreter. Sep 4, 2016
@astroshim astroshim changed the title [ZEPPELIN-1405] ConnectionPool for JDBCInterpreter. [WIP] [ZEPPELIN-1405] ConnectionPool for JDBCInterpreter. Sep 4, 2016
@astroshim
Copy link
Contributor Author

\cc @jongyoul I changed some codes to use Connection Pool that is org.apache.commons.dbcp2, please review.

@astroshim astroshim changed the title [WIP] [ZEPPELIN-1405] ConnectionPool for JDBCInterpreter. [ZEPPELIN-1405] ConnectionPool for JDBCInterpreter. Sep 5, 2016
@astroshim
Copy link
Contributor Author

I tested mysql, postgresql and hive.

@jongyoul
Copy link
Member

jongyoul commented Sep 6, 2016

Thanks! I'll look into it. btw, don't you have any problem when you were testing mysql?

@astroshim
Copy link
Contributor Author

Thanks @jongyoul and there was no problem running mysql.


private InterpreterResult executeSql(String propertyKey, String sql,
InterpreterContext interpreterContext) {
InterpreterContext interpreterContext) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@astroshim It's not recommended. you should change your IDE setting :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah.. okay i will

@astroshim
Copy link
Contributor Author

ping

@astroshim
Copy link
Contributor Author

re-trigger CI

@astroshim astroshim closed this Sep 9, 2016
@astroshim astroshim reopened this Sep 9, 2016
@astroshim
Copy link
Contributor Author

re-trigger CI

@astroshim astroshim closed this Sep 12, 2016
@astroshim astroshim reopened this Sep 12, 2016
@astroshim astroshim closed this Sep 22, 2016
@astroshim astroshim reopened this Sep 22, 2016
@astroshim
Copy link
Contributor Author

CI has passed. please review.

@asfgit asfgit closed this in b24491b Sep 28, 2016
pedrozatta pushed a commit to pedrozatta/zeppelin that referenced this pull request Oct 27, 2016
### What is this PR for?
This PR is for refactoring code for JDBCInterpreter.
There is no putting 'Connection' to 'propertyKeyUnusedConnectionListMap' anywhere in the original code.

### What type of PR is it?
Improvement

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1405

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: astroshim <[email protected]>

Closes apache#1396 from astroshim/ZEPPELIN-1405 and squashes the following commits:

b07e162 [astroshim] add checking connection is null
f6998c2 [astroshim] Merge branch 'master' into ZEPPELIN-1405
1862ae6 [astroshim] Merge branch 'master' into ZEPPELIN-1405
efc2bfc [astroshim] rebase
21217a7 [astroshim] fix indentation.
4d4f85c [astroshim] refactoring code of close()
9f1e368 [astroshim] replace ConnectionPool
4dabbcc [astroshim] wip) changing to use dbcp
12dd7cb [astroshim] remove propertyKeyUnusedConnectionListMap map
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants