Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

adding Node4 build to CI #60

Merged
merged 1 commit into from
Jun 17, 2016
Merged

Conversation

matzew
Copy link
Contributor

@matzew matzew commented Jun 1, 2016

et voila! Some parts are done for: RHMAP-6229

@matzew
Copy link
Contributor Author

matzew commented Jun 1, 2016

@david-martin @maleck13 mind taking a review ?

@david-martin
Copy link
Contributor

@matzew The change & passing builds look good.
@cianclarke can you suggest any happy path to verify that things are working ok on node 4? Or is the passing build enough?

@maleck13
Copy link
Contributor

maleck13 commented Jun 2, 2016

@matzew @david-martin There are some docs here that have a getting started example
http://testing.cbeng.skunkhenry.com/docs/guides/api_mapper.html

@matzew
Copy link
Contributor Author

matzew commented Jun 7, 2016

@maleck13 I tried to deploy my branch, in here:
http://testing.matzew-bf.skunkhenry.com/#services/4mtmp23oc3tcnvvron5632xo/apps/4mtmp23t5jez2en2ee6wkx2y/deploy

and I am getting this:

Deploy started for environment live
 Performing pre-deploy checks
 Pre-deploy checks passed
 Deploy operation scheduled
 Checking app runtime
 Runtime is ok. Creating app
 [stdout] Source already contains node_modules, not running npm
 Updating app state
 Starting app

 Failed to deploy app due to error undefined

 Error deploying cloud App to selected deployment target! 
''

@maleck13
Copy link
Contributor

maleck13 commented Jun 7, 2016

@matzew should it have the node_modules? Is this a dynofarm target or os3?

Source already contains node_modules, not running npm

This could cause issues if the node modules have any native dependency that was compiled on one OS and then deployed to a different one.

@matzew
Copy link
Contributor Author

matzew commented Jun 8, 2016

@maleck13 I have no clue about node_modules, so I am overasked

@cianclarke
Copy link
Contributor

Indeed, the tutorial Dave linked to above is a good "happy path" test.
Not sure why node_modules is in the git repo, but looks like we're getting confused on the .node_modules pre-commit hook that's in there for OpenShift? Not sure what that does..

@odra
Copy link

odra commented Jun 13, 2016

I started a deploy for node 0.10 and got this error:

 [stderr] Error: Command failed: npm WARN deprecated lodash@2.4.1: lodash@<3.0.0 is no longer maintained. Upgrade to lodash@^4.0.0.
 [stderr] npm WARN deprecated lodash@1.0.1: lodash@<3.0.0 is no longer maintained. Upgrade to lodash@^4.0.0.
 [stderr] npm WARN deprecated graceful-fs@1.2.3: graceful-fs v3.0.0 and before will fail on node releases >= v7.0. Please update to graceful-fs@^4.0.0 as soon as possible. Use 'npm ls graceful-fs' to find it in the tree.
 [stderr] npm ERR! fetch failed https://registry.npmjs.org/spdx-license-ids/-/spdx-license-ids-1.0.2.tgz
 [stderr] npm ERR! error rolling back Error: ENOTEMPTY, rmdir '/home/fh/.fh-npm-node010/cipdzq1tv000gi0zv3rnblf6x/node_modules/patternfly/node_modules/grunt-contrib-uglify/node_modules/uglify-js/node_modules/source-map/test'
 [stderr] npm ERR! error rolling back  patternfly@2.10.0 { [Error: ENOTEMPTY, rmdir '/home/fh/.fh-npm-node010/cipdzq1tv000gi0zv3rnblf6x/node_modules/patternfly/node_modules/grunt-contrib-uglify/node_modules/uglify-js/node_modules/source-map/test']
 [stderr] npm ERR! error rolling back   errno: 53,
 [stderr] npm ERR! error rolling back   code: 'ENOTEMPTY',
 [stderr] npm ERR! error rolling back   path: '/home/fh/.fh-npm-node010/cipdzq1tv000gi0zv3rnblf6x/node_modules/patternfly/node_modules/grunt-contrib-uglify/node_modules/uglify-js/node_modules/source-map/test' }
 [stderr] npm ERR! Error: 400 Bad Request
 [stderr] npm ERR!     at WriteStream. Error deploying cloud App to selected deployment target! 
''

Then I started a deploy for node 4 and it worked. Could this be some npm cache or file cleanup problem?

@david-martin
Copy link
Contributor

@odra It looks like a possibly transient issue. It might be good to see how reproducable it is, and if it's isolated to node4.
If it isn't easily reproduced, I don't think we should block this change.

@matzew
Copy link
Contributor Author

matzew commented Jun 16, 2016

@david-martin @odra So, could it be merged? or more investigations needed ?

@odra
Copy link

odra commented Jun 16, 2016

@matzew I wasn't able to get any errors in my cluster, so I think we can merge it.

@matzew
Copy link
Contributor Author

matzew commented Jun 16, 2016

@odra ok, I will leave it to @maleck13 or @david-martin :)

@@ -1,9 +1,12 @@
language: node_js
node_js:
- "0.10"
- "4.3"
- "4.4"
Copy link
Contributor

Choose a reason for hiding this comment

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

Version 4.4.2 should be enough rather than 2 versions of node4.
This is the version used in rhscl

docker run --rm registry.access.redhat.com/rhscl/nodejs-4-rhel7 node -v
v4.4.2

@matzew
Copy link
Contributor Author

matzew commented Jun 17, 2016

@david-martin fixed, rebased.

So, can be merged ?

@david-martin
Copy link
Contributor

@matzew sure, i'll merge

@david-martin david-martin merged commit f6ed9e6 into feedhenry-templates:master Jun 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants