feat: Ability to pass in odbc connection to transport

[abmusse]

Resolves #279 Resolves #282

[abmusse]

As written now closing of the odbc connection is based on if the odbcConnection transport object was set. When a connection is passed in we do not close it. Otherwise we create the connection, run the query, and close it. https://github.com/IBM/nodejs-itoolkit/pull/282#discussion_r560357904 posses a good question if we should allow closing of the connection to be configurable.

[abmusse]

Examples Using Existing ODBC Connection

Callbacks

const { Connection, CommandCall } = require('itoolkit');
const { parseString } = require('xml2js');
const odbc = require('odbc');

  odbc.connect('DSN=*LOCAL', (connectError, connection) => {
    if (connectError) { throw connectError; }
    const toolkit = new Connection({
      transport: 'odbc',
      transportOptions: {
        odbcConnection: connection,
      },
    });

    const command = new CommandCall({ type: 'cl', command: 'RTVJOBA USRLIBL(?) SYSLIBL(?)' });

    toolkit.add(command);
    toolkit.run((error, xmlOutput) => {
      if (error) { throw error; }
      parseString(xmlOutput, (parseError, result) => {
        if (parseError) { throw parseError; }
        console.log(result);
        // close odbc connection
        connection.close((closeError) => {
          if (closeError) { throw closeError; }
        });
      });
    });
  });

Promises

const { Connection, CommandCall } = require('itoolkit');
const { parseString } = require('xml2js');
const odbc = require('odbc');

async function main() {
  const connection = await odbc.connect('DSN=*LOCAL');
  const toolkit = new Connection({
    transport: 'odbc',
    transportOptions: {
      odbcConnection: connection,
    },
  });

  const command = new CommandCall({ type: 'cl', command: 'RTVJOBA USRLIBL(?) SYSLIBL(?)' });

  toolkit.add(command);
  // toolkit.run is asynchronous and invokes callback on completion
  //  therefore need to wrap this is call with a new promise when working with async/await 
  const p = new Promise((resolve, reject) => {
    toolkit.run((error, xmlOutput) => {
      if (error) { reject(error); }
      parseString(xmlOutput, (parseError, result) => {
        if (parseError) { reject(parseError); }
        resolve(result);
      });
    });
  });

  const result = await p;
  await connection.close(); // close odbc connection
  return result;
}

main().then((result) => {
  console.log(result);
}).catch((error) => {
  console.error(error);
});

[github-actions[bot]]

:wave: Hi! This pull request has been marked stale due to inactivity. If no further activity occurs, it will automatically be closed.

[alanseiden]

Is this PR still being worked on? It would help us encourage use of the odbc transport if the open/close performance penalty were no longer a concern.

[abmusse]

Its been a minute since we worked on this. I still would like to get this merged as it would enhance performance. Need to have a discussion with the team on the strategy for this change going forward.

[brandonp42]

Hi @abmusse, did you ever discuss this with the team? The concept of this change seems like a no-brainer to me but I haven't tested it yet.

[abmusse]

Hi @abmusse, did you ever discuss this with the team? The concept of this change seems like a no-brainer to me but I haven't tested it yet.

Hi @brandonp42 , Thanks for the ping Its been a year and his change has been left on the shelf unattended. We haven't discussed the points of concern due to other priorities.

IIRC, some the points I wanted to discuss with the team was:

• Is there a default timeout for odbc connection? If so is the time out configurable to keep the connection alive indefientely?

• How to handling "dead" connections? Should we just reconnect if the expected connection is "dead" Currently this PR does not make reconnect attempts. Maybe this could be handled by passing DSN / connection details in. Each call check if the passed in connection is connected. if not use the DSN / connection details to create the connection and continue.

• Also was considering adding promise support as mentioned here https://github.com/IBM/nodejs-itoolkit/issues/322#issuecomment-790845969 (For now though I think having the performance boost keep a connection open is higher priority) This would be more involved as some of transports don't support promises (ssh2 library)

Could you help test things out and give feedback if you encounter "dead" connections or other issues? Also, Does your use case keep the connection open for long time?

This would help move forward to getting this change merged in. If you agree I would re-base this PR with latest main branch. Then you can install this version of itoolkit with odbc changes with:

mkdir ~/test-itoolkit-keep-odbc-open
cd ~/test-itoolkit-keep-odbc-open
npm init -y
npm install "https://github.com/abmusse/nodejs-itoolkit#odbc-transport-use-existing-connection" --save

Here is an example passing in the odbc connection: https://github.com/IBM/nodejs-itoolkit/pull/320#issuecomment-763899232

[brandonp42]

Sure, I will try to do some testing with this. My use case is for incoming API connections via Express where we call our internal service programs to do things and then return data back to the caller. So for that we generally want to use connection pools that stay alive for performance. As part of that we're also running a custom version of XMLSERVICE that I've fixed some bugs in.

[abmusse]

FYI I've re-based this PR with the latest changes on the main branch.