Skip to content
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

Make it compatible with and contribute support for it within knex.js #3

Closed
tombburnell opened this issue Jan 21, 2015 · 27 comments
Closed
Assignees

Comments

@tombburnell
Copy link

We are considering migration from mysql to oracle but our application is currently heavily reliant on knex.js. Making node-oracle compatible with knex.js would make this a possibility for us.

Cross link for convenience: knex/knex#646

@tombburnell tombburnell changed the title Make it compatible and contribute support for it within knex.js Make it compatible with and contribute support for it within knex.js Jan 21, 2015
@cjbj cjbj self-assigned this Jan 21, 2015
@cjbj
Copy link
Member

cjbj commented Jan 21, 2015

@tombburnell we'll need help on this. Would you be able to dig into knex.js and assess what it would take?

@tombburnell
Copy link
Author

Unfortunately I don't get too much free time so it's unlikely. I'd suggest reaching out to the Knex.js developer(s) and make it clear you want to collaborate with them. They might be able to give you a blueprint to work against and guide you. I think starting knex integration would be the best way to get more people involved in using/contributing to the library.

@cjbj
Copy link
Member

cjbj commented Jan 25, 2015

@tombburnell from experience, there has to be user demand before one community will see the value of integrating their tech with another. And users will have to help maintain the code going forward. This means it is important for you as a user to also contact the Knex team. And also if you come across other Knex users who want Oracle support make sure I'm aware of the demand too.

@vschoettke
Copy link

@cjbj I implemented the node-oracle support for knex.js. Adaptation should be not so difficult, however the driver needs to support for Outparams because of needed workarounds for the Oracle behavior. I glanced over your documentation and have not seen support for it mentioned yet.

It would very helpful if Oracle can supply direct download urls for Oracle XE and the Instant Basic Client 12 for Linux X64, so that the oracle dialect is always properly tested (including pull requests) with TravisCI.

@cjbj
Copy link
Member

cjbj commented Jan 28, 2015

@vschoettke By 'outparams' do you mean support for Oracle's RETURNING INTO clause (see #4)? Node-oracledb 0.2 already has support for OUT and IN OUT binds from calls to Oracle's PL/SQL https://github.com/oracle/node-oracledb/blob/master/doc/api.md#outbind

Your point about TravisCI is valid.

@robertd
Copy link

robertd commented Jan 29, 2015

+1 on knex/oracle support

@pmarino90
Copy link

+1

@coderbuzz
Copy link

So, is https://github.com/tgriesser/knex/tree/master/lib/dialects/oracle ready to use?

*Edit, sorry that one for node-oracle https://github.com/joeferner/node-oracle

@atiertant
Copy link

+1

@tgriesser
Copy link

Yeah if anyone can help me out with this I'd love to get support in there - in particular the best way to get this setup with some sort of CI suite (travis or otherwise).

@tgriesser
Copy link

I just did some googling and came across this - https://github.com/cbandy/travis-oracle, looks promising

@vschoettke
Copy link

@tgriesser I already did the Oracle Travis CI integration a long time ago. (see the oracle-ci-intregration branch in my knex fork) The problem is not the CI integration but that Oracle XE is not publicly available for download. (You have to agree to the Oracle license.) That still hasn't changed yet.

@cjbj
Copy link
Member

cjbj commented Sep 24, 2015

Knex node-oracledb testers needed: See comment from @atiertant in knex/knex#990

@cjbj
Copy link
Member

cjbj commented Sep 24, 2015

/cc @tombburnell

@atiertant
Copy link

@cjbj at this time node-oracledb can be used with knex using 'oracledb' dialect.
but there is a problem with travis ci tests...
of course oracle database could be executed in a docker image but what about oracle instant client in travis to install node-oracledb ?
i seen https://github.com/sagiegurari/simple-oracledb tests but only node 6 can be tested...

@alandotcom
Copy link

@atiertant Here's an example of running tests with Oracle on CircleCI (the example is for strong-oracle, but the same can be done for oracledb). The Oracle Instantclient SDK needs to be downloaded from somewhere, in this case a read-only S3 bucket, and installed on the CircleCI VM running the test. The circle.yml setup takes advantage of CircleCI caching, so that the files only need to be downloaded once.

@sagiegurari
Copy link

@atiertant its not that I can only test with node 6.
I have full tests for simple oracledb that do not require a DB (I use mocks).
So I picked only the latest node version to ALSO run few tests against a real DB.
The reason I don't do it on all node versions is the time it takes to run the tests with docker.
So basically you can run tests on any node version you like starting from 0.10

@atiertant
Copy link

@sagiegurari oups sorry ! how do you install instantclient ?

a "how to" would be realy helpful

@sagiegurari
Copy link

take a look at the build.sh
https://github.com/sagiegurari/simple-oracledb/blob/81f78d4c7442130b9bdf24b9e4c3384d4569e840/project/build/integration/build.sh
it sets up 2 docker containers.
first is a container with oracle 11 db running (public docker file).
second is a container that is based on another public dockerfile that has node + npm + instant client, but I add another layer on top which basically install simple-oracledb + oracledb + mocha and I run few tests.

its important to be able to test your code without complex 3rd parties that is why I mock oracle and got tests for every function in my libs for all scenarios.
however, mock cannot be the only thing, and you got to do a bit of testing with real database, that is why I have few tests that cover the common scenarios with docker.
but i suggest to you that you see how you can run most tests without.

@atiertant
Copy link

hum so you need one docker container for each node version and one more for the database.
this could not be done on knex tests like that...
i was thinking about using a docker container for database and serve instantclient on local http
so top layer directly download instantclient from docker

@sagiegurari
Copy link

you can do that. personally I picked to use 2 docker images that already existed and just connect them.
but you can do 1 image that has everything. just more complex to setup that's all.
I'm sure if you do it, many will want to use it. probably me too.

@atiertant
Copy link

i never used docker but i could learn...
what about oracle license ?

@sagiegurari
Copy link

@cjbj can answer that one best.

@cjbj
Copy link
Member

cjbj commented Sep 6, 2016

@atiertant if you are installing node-oracledb on a machine (or image) that has an Oracle Database you don't need to install Instant Client. See https://github.com/oracle/node-oracledb/blob/master/INSTALL.md#instoh

Thanks for taking the lead on the Knex integration. Should we close this (#3) issue now?

Another Docker Oracle DB reference is: https://blogs.oracle.com/developer/entry/creating_and_oracle_database_docker which shows using Oracle's docker build images for the database: https://github.com/oracle/docker-images/tree/master/OracleDatabase

For Instant Client, have you seen @bchr02's https://github.com/bchr02/instantclient

Regarding the license: IANAL. Just check the license of what you download. Things downloaded from OTN tend to have licenses that are developer-friendly.

@cjbj
Copy link
Member

cjbj commented Sep 7, 2016

@atiertant
Copy link

@cjbj yes, this could be closed, but i'll open a new one : "Make it compatible with Travis CI"
i wouldn't like to install and use instantclient in docker, i would like docker to redistribute it on a local http server.
i seen in license: "We grant you a non-exclusive right and license to distribute the Programs, provided that you do not charge your end users for use of the Programs. Your distribution of such Programs shall at a minimum include the following terms in an executed license agreement between you and the end user"
so if i add the a License file in the docker container and add a ACCEPT_INSTANTCLIENT_LICENSE=true parameter in the docker command, could my docker container redistribute instantclient ?

@cjbj
Copy link
Member

cjbj commented Dec 3, 2016

I'm going to close this issue: for KNEX and Oracle, see (for example) https://github.com/tgriesser/knex/tree/master/src/dialects/oracledb

Thanks for making it all work!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants