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

feat: Add DataQueue class #151

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

markdirish
Copy link
Collaborator

Add a DataQueue class that can be called similar to the Java toolkit
Experiment with pulling out API data into a const variable

PR opened to discuss work thus far and solicit input into how to improve pulling out these classes a la the Java Toolkit

Add a DataQueue class that can be called similar to the Java toolkit
Experiment with pulling out API data into a const variable
lib/DataQueue.js Outdated
program.addParam(this.name, QRCVDTAQParameters.DataQueueName.type, { io: QRCVDTAQParameters.DataQueueName.io });
program.addParam(this.library, QRCVDTAQParameters.LibraryName.type, { io: QRCVDTAQParameters.LibraryName.io });
program.addParam(length, QRCVDTAQParameters.LengthOfData.type, { io: QRCVDTAQParameters.LengthOfData.io });
program.addParam('', `${this.maxLength}A`, { io: QRCVDTAQParameters.Data.io });
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We talked about how to do this for "structures" (the array) to get the len and then using setlen, but can I do the same for single values? And should I here, with the LengthOfData parameter?

lib/DataQueue.js Outdated
Comment on lines 5 to 12
// QRCVDTAQ Parameters
const QRCVDTAQParameters = {
DataQueueName: { io: 'in', type: '10A' },
LibraryName: { io: 'in', type: '10A' },
LengthOfData: { io: 'in', type: '5p0' },
Data: { io: 'out', type: '10A' },
WaitTime: { io: 'in', type: '5p0' }
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interested in what others think about pulling Parameter information out into a structure like this. Might work a little better after the this PR drops: #139

lib/DataQueue.js Outdated
Comment on lines 180 to 203
async _getDataQueueDescription(callback) {
const program = new ProgramCall('QMHQRDQD', { lib: 'QSYS' });

const receiverVariable = [
[0, '10i0'],
[0, '10i0'],
[0, '10i0']
];

program.addParam(receiverVariable, { io: 'out', len: 'varLen'}); // Receiver variable
program.addParam(0, '10i0', { setlen: 'varLen'}); // Length of receiver variable
program.addParam('RDQD0100', '8A'); // Format name
program.addParam(this.qualifiedName, '20A'); // Qualified data queue name

this.connection.add(program)
this.connection.run((error, xmlOutput) => {
if (error) {
return callback(error, null);
}

const result = xmlToJson(xmlOutput);
this.maxLength = result[0].data[2].value
return callback(null, result);
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Idea here is:

If we create a DataQueue using this API, we know what the max length is. But if we are consuming a DataQueue created elsewhere, we don't know the max size. So when we need to get the length the first time we call an API that requires us to specify the length of the receiver data

@ThePrez
Copy link
Member

ThePrez commented Mar 30, 2020

I like the idea, and if we can strive to be more like the revered Java Toolbox, that's great. However, now that we have the *DATA_QUEUE* SQL services, this is much less necessary
https://www.ibm.com/support/pages/ibm-i-services-sql
A Data Queue class and associated java types is still good for usability, so I'm not opposed to this enhancement, but want to doc that the SQL path is a food fallback if someone needs such function before this class is completed.

@github-actions
Copy link

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

@github-actions github-actions bot added the stale label Apr 30, 2020
@abmusse abmusse added the keep-open Exempts stale action from auto closing the issue/pr. label Apr 30, 2020
@github-actions github-actions bot closed this May 8, 2020
@abmusse abmusse mentioned this pull request May 8, 2020
@abmusse abmusse reopened this May 8, 2020
@ghost
Copy link

ghost commented Jul 14, 2020

I like the idea, and if we can strive to be more like the revered Java Toolbox, that's great. However, now that we have the *DATA_QUEUE* SQL services, this is much less necessary
https://www.ibm.com/support/pages/ibm-i-services-sql
A Data Queue class and associated java types is still good for usability, so I'm not opposed to this enhancement, but want to doc that the SQL path is a food fallback if someone needs such function before this class is completed.

I'd like to see this class added since the SQL Data queue service aren't support on 7.2. Our upgrade is planned, but with the world as it it that may be delayed.

@github-actions github-actions bot removed the stale label Oct 6, 2020
@kadler kadler linked an issue Oct 21, 2020 that may be closed by this pull request
@markdirish
Copy link
Collaborator Author

I have a LUG customer concerned about the deprecation of iDataQueue, I'm going to make sure this PR is up to date and refresh any questions I have, then it might be a good idea to throw some developer minutes/hours at it to get it landed.

Signed-off-by: Mark Irish <mirish@ibm.com>
Signed-off-by: Mark Irish <mirish@ibm.com>
@markdirish
Copy link
Collaborator Author

Ok, I have tested the latest push:

Passing:

  • Creating a DataQueue with new DataQueue works
  • Creating a DTAQ on the system with create function works
  • Deleting a DTAQ on the system with delete function works
  • Receiving data with receive works
  • Sending data with send works

Failing:

  • When calling receive, if you don't pass a length, it will try to call QMHQRDQD to get information about the DTAQ. I am not sure how to add the params correctly to get it to work... particularly the receiver variable, type: 'ds', and the like.

I see I have some linting failing, I might have disabled eslint on my VSCode at some point. Will reenable and ensure Linting and DCO bot are happy. No idea why GitHub Actions tests are failing, I think I haven't touched anything outside of my file.

Lingering questions:

  • I see that xmlToJson was deprecated, along with all of the other functions we had. I have adjusted the class to return XML, but it seems like it would be best to return JSON? Since this is the first helper(?) class, maybe we can talk about how to do that best.
  • I see that everything is returning callback functions, and I have emulated that behavior. It is possible to have functions use callbacks or Promises, similar to how I do in the odbc package. Just check to see if a callback was passed as the final parameter, and if not, go to a Promise workflow. This would ensure API stability with anyone currently using the package, but would also allow developers to use the more modern Promise syntax.

Copy link
Member

@abmusse abmusse left a comment

Choose a reason for hiding this comment

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

No idea why GitHub Actions tests are failing, I think I haven't touched anything outside of my file.

The build machine is currently down so fails to run the integration tests. We will need to create some tests for the new class though.

The PR linter check is failing because the title does not follow conventional commit type

I would suggest feat: Add DataQueue class

lib/DataQueue.js Outdated Show resolved Hide resolved
* @param {number} maxLength
* @param {function} callback
*/
async create(maxLength, callback) {
Copy link
Member

Choose a reason for hiding this comment

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

Should these methods be defined as async? I'm thinking this could lead to synchronization issues. Async function return a promise and within the function we call connection.run with is asynchronous function using the callback pattern. So we wouldn't wait for connection.run to complete before returning the promise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So this goes with Issue #322 . I have always declared them as async since that's what they are. I wasn't aware the spec said I had to return a Promise, just that "Async functions can contain zero or more await expressions." Because of the latter I don't think using callbacks internally is a big deal. Will have to dwell on it, but maybe is a reason people don't mix callbacks/Promises in the same function

@markdirish markdirish changed the title WIP: [issue97] Add DataQueue class feat: Add DataQueue class Mar 4, 2021
Signed-off-by: Mark Irish <mirish@ibm.com>
@maghirian
Copy link

Ok, I have tested the latest push:

Passing:

  • Creating a DataQueue with new DataQueue works
  • Creating a DTAQ on the system with create function works
  • Deleting a DTAQ on the system with delete function works
  • Receiving data with receive works
  • Sending data with send works

Failing:

  • When calling receive, if you don't pass a length, it will try to call QMHQRDQD to get information about the DTAQ. I am not sure how to add the params correctly to get it to work... particularly the receiver variable, type: 'ds', and the like.

I see I have some linting failing, I might have disabled eslint on my VSCode at some point. Will reenable and ensure Linting and DCO bot are happy. No idea why GitHub Actions tests are failing, I think I haven't touched anything outside of my file.

Lingering questions:

  • I see that xmlToJson was deprecated, along with all of the other functions we had. I have adjusted the class to return XML, but it seems like it would be best to return JSON? Since this is the first helper(?) class, maybe we can talk about how to do that best.
  • I see that everything is returning callback functions, and I have emulated that behavior. It is possible to have functions use callbacks or Promises, similar to how I do in the odbc package. Just check to see if a callback was passed as the final parameter, and if not, go to a Promise workflow. This would ensure API stability with anyone currently using the package, but would also allow developers to use the more modern Promise syntax.

Hey Mark....Regarding the lingering question on deprecated xmlToJson...If you have not figured it out by now :) use xml2js:

const { parseString } = require('xml2js');

connection.run((error, xmlOutput) => {
if (error) {
throw error;
}
parseString(xmlOutput, (parseError, result) => {
if (parseError) {
throw parseError;
}
console.log(JSON.stringify(result));
});
});_

Hope this helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep-open Exempts stale action from auto closing the issue/pr.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extract Data Queue functions in to a real DataQueue class
4 participants