-
Notifications
You must be signed in to change notification settings - Fork 133
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
SNOW-748680: Update node fips docker image and re-add fips test #500
Conversation
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.
LGTM
The "nodejs-centos7-fips" build is failing due to a syntax error: https://github.com/snowflakedb/snowflake-connector-nodejs/actions/runs/5073384764/jobs/9133365714?pr=500#step:6:272 For await loops are available from node 10.3 and above: https://node.green/#ES2018-features-Asynchronous-Iterators-for-await-of-loops. The 14.17.0 version is already specified in the docker file but looks like it wasn't picked up The node version of the "nodejs-centos7-fips" image needs to be updated similar to how the "nodejs-centos7-node12" image was updated for the test to pass |
@sfc-gh-kdama ^^^^ |
a7c56d9
to
2fc1108
Compare
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.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #500 +/- ##
=======================================
Coverage 84.80% 84.80%
=======================================
Files 56 56
Lines 5199 5199
=======================================
Hits 4409 4409
Misses 790 790 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Update the node version on the docker image to fix the following error:
ERROR: npm v9.6.5 is known not to run on Node.js v12.0.0. This version of npm supports the following node versions:
^14.17.0 || ^16.13.0 || >=18.0.0. You can find the latest version at [https://nodejs.org/.](https://nodejs.org/)
Re-add fips test for centos7