-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add connection pooling and resource cleanup #246
Comments
And it is indeed more appropriate for a separate wrapper module. |
Thanks @ericwaldheim. I didn't realize that strong-oracle had connection pooling, it's not in the documentation that I could find, but looking through their code I see it now. Have you had success using strong-oracle's pooling? I don't see anything regarding how to configure it. |
Also, somewhat related, what is the relationship between strong-oracle and this module, if there is one? I'm assuming they're forks one way or the other, but there doesn't seem to be any reference in either module. All the info I can find on the web suggests that node-oracle is the way to go and strong-oracle is never mentioned. |
If we're mentioning other drivers, https://github.com/oracle/node-oracledb also has connection pooling. |
@cjbj wow, hot off the presses, I didn't realize oracle was creating their own drivers for node. that's great, I will try it out. |
strong-oracle is a fork of this module (see: https://github.com/strongloop/strong-oracle/blob/84886677557e0aeb8d1eb2bd4c2ebfe88d3a2e4e/NOTICE). Prior to the fork, @raymondfeng was contributing some very helpful commits to this module. Strong-oracle has been actively maintained by both raymondfeng and bnoordhuis, and is almost certainly more robust than this codebase at this time. The main downside with strong-oracle is its slightly more restrictive (and certainly more complex) license: https://github.com/strongloop/strong-oracle/blob/84886677557e0aeb8d1eb2bd4c2ebfe88d3a2e4e/LICENSE. Thanks for chiming in, @cjbj! An oracle-backed driver is probably the safest bet, especially given, in my opinion, the complexity of working with Oracle OCCI and its occasionally-lacking documentation. |
@theothermattm we did give a heads up to some people, including @joeferner and the StrongLoop crew, and we demoed a very early implementation last year. Yesterday was the big release. We use the database's C API not the C++ one so there are significant architectural differences underneath. |
Wow this is an immensely helpful thread. I'm glad I asked. Thank you @raztus and @cjbj. |
@raztus send any OCCI questions my way and I'll make sure to find an answer. |
Also, to address your original question, @theothermattm, about "The README mentions that connection pooling support will eventually be added," I think I put that in because I was planning on merging the connection pooling that raymongfeng was working on in the strong-oracle fork. At the time I think he kept the MIT license from this library, so merging it would have been clearly acceptable (and still might be, going back retroactively to the commits licensed as such.) Now it's less clear. @cjbj Thanks for the offer! I've all but abandoned coding on this project, though I still follow it and have been meaning to address the outstanding pull requests. With an oracle-sanctioned solution now available, I may not even continue following this. |
@raztus it does seem to make sense to move towards the oracle version. Does it make sense to add a note to the README.md for this and merge it into master sooner rather than later? |
also, I feel like I should go through all the old stack overflow posts I read to finally arrive at node-oracle and let them know about node-oracledb ;) |
I'll have to leave that up to @joeferner, since this is really his baby, and I've just stepped up to help with some of the maintenance--though I've defaulted on that effort. Before you start preaching it to the masses, you might consider installing and playing with it first to make sure it fits your needs. |
Good point, not so much preaching though as just communicating the news. It was pretty hard for me to find good info on the node.js/oracle topic. |
It would be great to have connection pooling built into the node-oracle module along with proper cleanup of resources.
What I mean is, something like this (very rough):
Additionally, if promises are used, resource cleanup can easily be done when the execution finishes (connection.close()) as well as destroying a connection if there is an error.
The README mentions that connection pooling support will eventually be added, however, I couldn't find any issue or documentation about this. I apologize if I missed something.
I'm not sure what the plans are for this, but I have some code that uses Javascript Promises and the node-pool module (https://github.com/coopernurse/node-pool) to achieve connection pooling along with (what I hope) is proper closing of connections when executing queries. I could abstract it out and and contribute it to the module if this is something the committers might want to include in this module.
If it's more appropriate for a separate wrapper module, I can see that too, especially since I know not everyone uses Promises.
Also, I didn't see a contributing.md so I hope this is an ok way to engage the community about this.
The text was updated successfully, but these errors were encountered: