-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Yarn transition work-in-progress #2659
Conversation
Running |
Hmmm, |
@ryzokuken any ideas? |
@jywarren who throws this error? Yarn? If we're on Node 0.x, we should definitely migrate. Node 4 has reached EOL, and ideally, we should jump to atleast Node 8. |
Yes I tried to set a higher node version... But it didn't work! See the
issue I linked to above for yarn... Thanks!
…On Sat, Apr 28, 2018, 8:38 AM Ujjwal Sharma ***@***.***> wrote:
@jywarren <https://github.com/jywarren> who throws this error? Yarn?
If we're on Node 0.x, we should definitely migrate. Node 4 has reached
EOL, and ideally, we should jump to atleast Node 8.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2659 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ1ELF3PdhiVxx5oEbizxZNpe-2Ezks5ttGKSgaJpZM4TnkO4>
.
|
But travis allows you to use yarn. All you need to do is to check-in the yarn lockfile (which we should, and follow a policy similar to Gemfile.lock). |
Can you add a commit to this PR? Thanks!
…On Sat, Apr 28, 2018, 9:15 AM Ujjwal Sharma ***@***.***> wrote:
But travis allows you to use yarn. All you need to do is to check-in the
yarn lockfile (which we should, and follow a policy similar to
Gemfile.lock).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2659 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ69__zWTmN60NcPBTFTaz-TrCS3vks5ttGtagaJpZM4TnkO4>
.
|
Trying to do more research in https://blog.travis-ci.com/2016-11-21-travis-ci-now-supports-yarn and add a |
Hmm, looks like nvm is installing node 6 outside the container but not inside? |
maybe something like this? https://stackoverflow.com/questions/25899912/install-nvm-in-docker |
OK, locally i get:
|
Getting closer -- i think we need help from @icarito maybe...
|
I would rather we used prepackaged node rather than nvm if possible there are debian packages we can use or ideally a ready made base image. https://hub.docker.com/ is really slow for me but I'd look for an image with the right Node version and then install the ruby packages from the distro. |
maybe we can switch to one of these: https://hub.docker.com/_/node/ |
Hmm, interesting:
I don't see |
Bundler version issue maybe? rubygems/bundler#4467 https://stackoverflow.com/questions/34276324/travis-reports-odd-message-of-corrupted-gemfile-lock |
Thanks for enduring build experiments with me. Thinking over this again I think the best approach is to return to Ruby base image as that is what ruby-node's Dockerfile does and install the desired Node version on top. We really shouldn't think of nvm or rvm in containers as containers are disposable by nature and get rebuilt with the exact desired versions, generally no need for version managers except at build time. |
That's fine, i'm not wedded to |
@icarito -- any suggestions on this? |
Sooner rather than later we will need to upgrade our container to Stretch. |
OK, so let's try first opening a new PR for the Stretch port, then return to this. @ryzokuken if you can try this out, just be sure to link back to here so there's a next step. |
Seeing:
|
@jywarren makes sense, it's bad if we're at Node v0.10.x, let's switch to Node v10.x instead. |
I'm fine with that! If you can move this forward at all, I'd be very
grateful. I'm just trying to move ahead with your excellent suggestion of
moving to Yarn, and am stuck here. Happy to accept help from anyone who can
get this to pass! Thanks!
…On Sun, Jun 10, 2018 at 3:57 PM, Ujjwal Sharma ***@***.***> wrote:
@jywarren <https://github.com/jywarren> makes sense, it's bad if we're at
Node v0.10.x, let's switch to Node v10.x instead.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2659 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ7ysjEJQSavzgwBlrm8_JD8k6fBBks5t7XobgaJpZM4TnkO4>
.
|
This is great. 🎉 Can you give us a checklist on anything that has to be done on production for this to work? Or do you and @icarito anticipate that it'll run smoothly. And one more thing - have you been able to double check that the most current bower.json file has been copied over (with accurate version numbers) to package.json? |
Yeah I will check the bower.json file again to see if any libraries are missing. Other than that I have made it to work smoothly in production but it would be great if @icarito can review this once. |
super, awesome.
…On Fri, Sep 7, 2018 at 11:17 AM Sourav Sahoo ***@***.***> wrote:
Yeah I will check the bower.json file again to see if any libraries are
missing. Other than that I have made it to work smoothly in production but
it would be great if @icarito <https://github.com/icarito> can review
this once.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2659 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ9YTx82qSb6WMsEKv7xl3a8d390dks5uYo3mgaJpZM4TnkO4>
.
|
All the packages are according to bower.json except the leaflet-environmental-layers as yarn doesn't have 1.0.1 version of it. So, we have to take the 1.0.0 version of it. |
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.
Other than the build order, this looks fine to me! Congratulations, this is a long effort!
Thanks again.
.travis.yml
Outdated
#language: node_js | ||
#node_js: '6' | ||
#cache: yarn # This caches `$HOME/.yarn-cache` | ||
|
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.
Please remove these comments unless they help somehow!
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.
Ok I wiil remove these
Makefile
Outdated
docker-compose exec web rake assets:precompile | ||
docker-compose exec web rake tmp:cache:clear | ||
docker-compose down --remove-orphans | ||
rm -f ./tmp/pids/server.pid | ||
docker-compose up -d | ||
docker-compose exec -T web yarn --ignore-engines --ignore-scripts --modules-folder public/lib |
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.
You've removed the javascript package installation from line 10/11 and put it into line 15. This effectively waits until after rebooting the Rails app to get going with Javascript installation. So if Javascript installation fails, you've already restarted the app possibly broken by missing Javascript dependencies. If you move this line to line 10, then if yarn fails the app is still running fine and no harm was done. If you agree please do so! Thanks!
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.
Ok I will move the yarn installation command to line 11. Thanks
Makefile
Outdated
docker-compose run web rake assets:precompile | ||
rm -f ./tmp/pids/server.pid | ||
docker-compose up -d | ||
docker-compose exec -T web yarn --ignore-engines --ignore-scripts --modules-folder public/lib |
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.
Again here, you wait till after deploying the container to install Javascript packages, I think the same applies here. Thanks!
@jywarren This morning it came to me that you may find it won't deploy cleanly with |
Yeah right @icarito putting yarn before restarting the container is giving error as you can see in https://jenkins.laboratoriopublico.org/job/Plots-Unstable/296/console . So, I guess best is to use the yarn command after starting the container. What do you think @icarito. |
Thanks @Souravirus for giving it a try! I think this is pinpointing an issue we've been having with deployment to production being somewhat unstable when upgrading stuff like Ruby. We've been using |
I think this is working fine in unstable now! @jywarren and the docker-compose bit should make it reliable to upgrade onwards. |
OK, shall I merge this in then? SUPER!!! |
Yeah its ready @jywarren |
🎉 !!! |
This is live!!!!! |
We have to change the documentation for the change from bower to yarn. I guess making a first-timer issue of it will work. |
I guess we can make it a code-in task too. What do you think @jywarren |
That'd be great!!!
…On Tue, Sep 11, 2018 at 6:07 PM Sourav Sahoo ***@***.***> wrote:
I guess we can make it a code-in task too. What do you think @jywarren
<https://github.com/jywarren>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2659 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ27_B3i3Q_i7nkPVQmPkrfyCLY-Rks5uaDQNgaJpZM4TnkO4>
.
|
* Yarn transition work-in-progress (help-wanted) * switch to yarn attempt * changed root location of packages * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update Dockerfile * Update .travis.yml * Update Dockerfile * Update Dockerfile * Update Dockerfile * Remove reference to unneeeded installations Keep minimal. * Still more tweaking for switching base OS image * Add .dockerignore Trying with ignored Gemfile.lock * Update Dockerfile * Update Dockerfile * Update Dockerfile * Added yarn.lock * Added phantomjs-prebuilt to package.json and yarn.lock * Removed separated installation of phantomjs-prebuilt * Run forced installation of yarn * Removed bower.json and .bowercc * Added noty and jquery-confirm * Modifications in Dockerfile * Added yarn install in .travis.yml * Added lel packages to package.json * Added yarn command in .gitlab.yml * Deleted bower command from .gitlab-ci.yml * Changes in .gitlab-ci.yml * Updated yarn according to bower * Deprecated lel version * Changes in Makefile * Removed bower from Makefile * Added rack-test to Gemfile * Modified Dockerfile according to current version * Changes in Gemfile.lock related to ruby 2.4.4 * Makefile changes related to ruby 2.4.4 * Removed bundle install command in .travis.yml * Added i18n Gem to Gemfile * Changes in Dockerfile related to npm error Hostname/IP doesn't match certificate's altnames * Changes in Dockerfile related to jobs option of bundle install * Removed Gemfile.lock in .dockerignore * Changes in .dockerignore * Changes in .dockerignore * Added Gemfile.lock in Dockerfile * Updated Gemfile.lock * Changes in .dockerignore * Modified way of installing bundler * Updated Dockerfile * Some changes in Dockerfile * Some changes * Changes in package.json * Removed comments in .travis.yml * Redeploy independent container * Use bash command line
Solves #1215