jdbc URL string support #34
Conversation
|
f548602 is the change proposed by @corradomio This PR is easier for contributors to co-author and collaboration on bringing standard URL string support for nebula-jdbc. |
|
We need help to review/polish/refactor this change. |
| } | ||
|
|
||
| // close the current pool that uses '127.0.0.1' (WHY????) | ||
| this.nebulaPool.close(); |
There was a problem hiding this comment.
maybe it should be refactored.
If url is the param of connect, then when get driver, there's should't init the pool, just init the pool in connect function.
Or give the url to driver, and init the connection pool in getDriver function.
The default client address 127.0.0.1 is unreasonable.
There was a problem hiding this comment.
totally agreed, would be better to remove this one.
There was a problem hiding this comment.
@Nicole00 I'm sorry but i saw line 180 the customizedAddressList will replace the poolProperties's addressList , so in my opinion the default IP 127.0.0.1 will by replaced by the customizedAddressList ?
There was a problem hiding this comment.
Yeah, it will be replaced by customizedAddressList only when user call new NebulaDriver(address), but it's not the standard way to use jdbc. Almost usage is Class.forName() and it will call the empty construct function, so the address will be 127.0.0.1.
We can remove the customizedAddressList and put the user address in url, which will be more common.
|
Another thing is, using the common way to load driver class won't get it registered yet. |
| for (String address : addressesList) { | ||
| String[] addressInfo = address.split(":"); | ||
| if(addressInfo.length != 2){ | ||
| throw new SQLException(String.format("url [%s] is invalid, please make sure your url match thr format: \"ip:port\".", url)); |
There was a problem hiding this comment.
Ha, thanks! Should be /thr/the/😊
As discussed in #27