-
Notifications
You must be signed in to change notification settings - Fork 38
feat: Add DataQueue class #151
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
base: master
Are you sure you want to change the base?
Conversation
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 }); |
There was a problem hiding this comment.
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
// 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' } | ||
} |
There was a problem hiding this comment.
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
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); | ||
}); |
There was a problem hiding this comment.
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
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 |
👋 Hi! This pull request has been marked stale due to inactivity. If no further activity occurs, it will automatically be closed. |
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. |
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 <[email protected]>
Signed-off-by: Mark Irish <[email protected]>
Ok, I have tested the latest push: Passing:
Failing:
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:
|
There was a problem hiding this 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
* @param {number} maxLength | ||
* @param {function} callback | ||
*/ | ||
async create(maxLength, callback) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Signed-off-by: Mark Irish <[email protected]>
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) => { Hope this helps. |
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