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

WIP: initial pure js impl for IBM i AIX variant #11

Merged
merged 5 commits into from Feb 25, 2019
Merged

WIP: initial pure js impl for IBM i AIX variant #11

merged 5 commits into from Feb 25, 2019

Conversation

ThePrez
Copy link
Contributor

@ThePrez ThePrez commented Feb 23, 2019

Creating as draft because the necessary db2util command has not yet been officially delivered by IBM. Hoping to address concerns in issues #5/#10.

The exception cases result in a UnhandledPromiseRejectionWarning but that seems in line with other implementations.

Review is welcome and appreciated

@ThePrez ThePrez marked this pull request as ready for review February 23, 2019 19:31
@ThePrez
Copy link
Contributor Author

ThePrez commented Feb 23, 2019

@patrickhrastnik , please review. Thanks!

@ThePrez ThePrez changed the title initial pure js impl for IBM i AIX variant WIP: initial pure js impl for IBM i AIX variant Feb 23, 2019
@ThePrez
Copy link
Contributor Author

ThePrez commented Feb 23, 2019

Sorry, this is still a WIP for reason initially stated. This was my first time trying GitHub's new "Draft PR" function, and apparently saying it's "ready for review" makes it no longer a draft. Sorry about that!

@silverwind
Copy link
Owner

This is much better. So I take it db2util is not available on other AIX variants, only IBM i? Is the reason it's not in PATH because it's so new?

@patrickhrastnik
Copy link
Contributor

Changes approved, works as expected on our IBM i
@ThePrez sent me a compiled version of db2util, which I deployed to our system. With that package, this code works fine.
Although, it should work fine with command db2util instead of the full path - I tested it in the shell, and since /QOpenSys/pkgs/bin is in PATH (by default?), it finds the command.

bash-4.4$ which db2util
/QOpenSys/pkgs/bin/db2util

@ThePrez
Copy link
Contributor Author

ThePrez commented Feb 25, 2019

Correct. This only works for the IBM i variant and not for the mainline AIX.
Once this lands, we should keep the existing AIX issue open or (my preference) close it and open a new issue specifically for "mainline AIX"

db2util will be shipped in /QOpenSys/pkgs/bin which is not in the default PATH for IBM i. Add-on tools aren't delivered in the default PATH for various reasons. I suspect @patrickhrastnik's environment has been configured to pick up node and therefore db2util unqualified, but that's not a requirement. I think it's safest to fully-qualify.

@silverwind silverwind merged commit 57a75d3 into silverwind:master Feb 25, 2019
@silverwind
Copy link
Owner

Merged and did a few minor tweaks in 4ac57d4. Would you mind giving latest master one more test before I release it?

@ThePrez
Copy link
Contributor Author

ThePrez commented Feb 25, 2019

Testing looks good to me. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants