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

Issue #782 : Ruby version upgraded to 2.7.2. #897

Merged
merged 6 commits into from
Mar 16, 2021

Conversation

sharvy
Copy link
Contributor

@sharvy sharvy commented Mar 4, 2021

Why

To include security fix in Webrick and many other changes.
Resolves #782

How

  • Update Gemfile and ruby-version files
  • Upgraded Node to 14+ and Webpacker to 5+.
  • Update Development Dockerfile & Production Dockerfile
  • Change CircleCI Config
  • Change Ansible Config
  • Build locally, run tests bin/dev/test, make sure all Green
  • Make sure CircleCI is working

Pre-Merge Checklist

  • Documentation and comments have been added to the codebase where required
  • Outstanding questions and concerns have been resolved

@h-m-m
Copy link
Collaborator

h-m-m commented Mar 5, 2021

Hi, @sharvy! I'm willing to spend some time looking into the test failures, that node error message. I can think of a couple of places to start adjusting for this, though I'm not 100% what's the best fix long-term. Do you have a good feel for this yourself?

@sharvy
Copy link
Contributor Author

sharvy commented Mar 5, 2021

@h-m-m,

We are using node 12.x. But all the prebuilt images (Docker, CircleCI) have node 14.x with Ruby 2.7.2.

One solution could be: Docker has ruby:2.7.2-alpine.3.12, but CircleCI doesn't have any prebuilt image with ruby 2.7.2 & node 14.x. So we can install nvm and swap node version in CircleCI.. I don't think that's a good solution for the long run.

Another solution could be: Upgrading node to 14.x in our project. Best for the long run, according to my opinion.

I went with the later solution, and everything is working perfectly.

@grenewode
Copy link
Contributor

Another nice thing about doing this is that it would allow for easy use & packaging on NixOS, which packages only ruby 2.7.2

@solebared
Copy link
Collaborator

This looks great @sharvy 💥
I'm going to deploy your branch to http://mutual-aid-test.herokuapp.com/ just to ensure it deploys ok.
Anything else needed before we merge?

@sharvy
Copy link
Contributor Author

sharvy commented Mar 9, 2021

@exbinary It looks good to me. 🙂
Good luck with the deployment, hope everything works as expected.

@solebared
Copy link
Collaborator

Hmm, so it deploys w/o error but loading any page generates the following error:

2021-03-11T05:32:57.002492+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443] ActionView::Template::Error (Webpacker can't find application.css in /app/public/packs/manifest.json. Possible causes:
2021-03-11T05:32:57.002493+00:00 app[web.1]: 1. You want to set webpacker.yml value of compile to true for your environment
2021-03-11T05:32:57.002493+00:00 app[web.1]: unless you are using the `webpack -w` or the webpack-dev-server.
2021-03-11T05:32:57.002493+00:00 app[web.1]: 2. webpack has not yet re-run to reflect updates.
2021-03-11T05:32:57.002494+00:00 app[web.1]: 3. You have misconfigured Webpacker's config/webpacker.yml file.
2021-03-11T05:32:57.002494+00:00 app[web.1]: 4. Your webpack configuration is not creating a manifest.
2021-03-11T05:32:57.002495+00:00 app[web.1]: Your manifest contains:
2021-03-11T05:32:57.002495+00:00 app[web.1]: {
2021-03-11T05:32:57.002495+00:00 app[web.1]: }
2021-03-11T05:32:57.002496+00:00 app[web.1]: ):
2021-03-11T05:32:57.002496+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443]      6:   <meta name="viewport" content="width=device-width, initial-scale=1" />
2021-03-11T05:32:57.002515+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443]      7:   <script src='https://api.mapbox.com/mapbox-gl-js/v1.12.0/mapbox-gl.js'></script>
2021-03-11T05:32:57.002516+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443]      8:
2021-03-11T05:32:57.002516+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443]      9:   <%= stylesheet_pack_tag 'application' %>
2021-03-11T05:32:57.002517+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443]     10:   <%= javascript_pack_tag 'application' %>
2021-03-11T05:32:57.002517+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443]     11: </head>
2021-03-11T05:32:57.002517+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443]
2021-03-11T05:32:57.002518+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443] app/views/layouts/_head.html.erb:9
2021-03-11T05:32:57.002518+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443] app/views/layouts/application.html.erb:3
2021-03-11T05:32:57.002525+00:00 app[web.1]: [b9e6c950-ee4f-47e4-8927-9b0e49890443] app/controllers/application_controller.rb:25:in `switch_locale'

Looks like possibly a webpack configuration error? Could be something broke on the webpacker upgrade.
I'll take a deeper look tomorrow but wanted to give you the update in the meantime.
🤔

@sharvy
Copy link
Contributor Author

sharvy commented Mar 12, 2021

@exbinary I think I figured it out. application.sass was not getting compiled and application.css was not getting generated, since key resolved_paths in no more valid in Webpacker versions >= 5. I changed the key to additional_paths and pushed a commit. Hope it will fix the issue. Can you give it a try?

@solebared
Copy link
Collaborator

solebared commented Mar 12, 2021

@sharvy , appreciate you following up on this and good catch! Unfortunately that change itself didn't fix the issue.

Good news is i believe i've found the two necessary fixes. I have a separate PR to upgrade just webpacker so i've implemented the fixes there. See the two most recent commits (8/12). That branch now seems to work on heroku.

Feel free to cherry-pick (or reimplement) those two changes into your branch. I think this'll cause less merge conflicts than merging my PR first but i'm happy to do it that way if you prefer.

Btw, for this and future troubleshooting, i was able to recreate the issue locally by running:

rm -rf public/packs
NODE_ENV=production RAILS_ENV=production bin/rails assets:precompile

At first, that showed the issue with webpack-cli. Once i upgraded that, then a compilation error surfaced which prompted the node-fetch addition.

@sharvy
Copy link
Contributor Author

sharvy commented Mar 12, 2021

@exbinary I have picked the changes from your PR. 🙂
Everything is working as expected, locally and on docker.

Copy link
Collaborator

@solebared solebared left a comment

Choose a reason for hiding this comment

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

Looks great! 💥 🙏🏾

@solebared solebared merged commit 6f47d02 into rubyforgood:main Mar 16, 2021
This was referenced Mar 16, 2021
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.

Upgrade to ruby 2.7.2
4 participants