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.

Sign up for GitHub

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

Jump to bottom

chore: upgrade ruby to 3.1.3 #5555

Merged
merged 32 commits into from
Jan 24, 2023
Merged

chore: upgrade ruby to 3.1.3 #5555

merged 32 commits into from
Jan 24, 2023

Conversation

vishnu-narayanan
Copy link
Member

@vishnu-narayanan vishnu-narayanan commented Oct 5, 2022 •
edited
Loading

Pull Request Template

Description

  • Upgrade ruby version from 3.0.4 to 3.1.3
  • This change would enable us to use the latest heroku-22 stack

Fixes https://github.com/chatwoot/product/issues/594

Type of change

Please delete options that are not relevant.

  • Breaking change (fix or feature that would cause existing functionality not to work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • PR checks
  • Tested on a self hosted installation

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@vishnu-narayanan
Copy link
Member Author 

Run bundle exec rake db:create
You don't have net-smtp installed in your application. Please add it to your Gemfile and run bundle install
rake aborted!
LoadError: cannot load such file -- net/smtp

@vishnu-narayanan
Copy link
Member Author

https://stackoverflow.com/questions/70500220/rails-7-ruby-3-1-loaderror-cannot-load-such-file-net-smtp
rails/rails#42366

@sojan-official sojan-official mentioned this pull request Dec 8, 2022
Closed
9 tasks
@sojan-official sojan-official self-assigned this Dec 8, 2022
@netlify
Copy link

netlify bot commented Dec 8, 2022 •
edited
Loading

✅ Deploy Preview for chatwoot-storybook canceled.

Name Link
🔨 Latest commit 6783826
🔍 Latest deploy log https://app.netlify.com/sites/chatwoot-storybook/deploys/63cfb3f356307100095ea2d0

@sojan-official
Copy link
Member

sojan-official commented Dec 8, 2022 •
edited
Loading

@vishnu-narayanan The circle builds are a success for this now. Should we upgrade the version for cwctl as well on this PR / or take it as a separate issue ?

Also, in the case of cwctl how do we handle the upgrade process?

  • might as well upgrade to 3.1.3 over this PR

sojan-official
sojan-official reviewed Dec 8, 2022
View reviewed changes
Copy link
Member

@sojan-official sojan-official left a comment

Choose a reason for hiding this comment

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

remove change from yarn lock

@vishnu-narayanan
Copy link
Member Author

vishnu-narayanan commented Dec 9, 2022 •
edited
Loading

@sojan-official

@vishnu-narayanan The circle builds are a success for this now. Should we upgrade the version for cwctl as well on this PR / or take it as a separate issue ?

cwctl will not need any changes.

Also, in the case of cwctl how do we handle the upgrade process?

cwctl will read the .ruby-version and upgrade the ruby if needed. No change is required here.

might as well upgrade to 3.1.3 over this PR

+1

@sojan-official
Copy link
Member

We can directly go for 3.2 :D https://www.ruby-lang.org/en/news/2022/12/25/ruby-3-2-0-released/

@sojan-official sojan-official changed the title chore: upgrade ruby to 3.1.2 chore: upgrade ruby to 3.2.0 Jan 9, 2023
@sojan-official
Copy link
Member

note: make sure to update the reference to the node version across the repo, like cwctl etc. also ensure that we have a pathway to upgrade node version in those installations

@vishnu-narayanan
chore: ping docker version to alpine3.16 for nodev16.x
f31a0c6 
Starting with Node 17, nodejs switched to OpenSSL3. The docker builds
are installing node18.xx with alpine-3.1.3.

From Node.js 17's announcement post:

    If you hit an ERR_OSSL_EVP_UNSUPPORTED error in your application
with Node.js 17, it’s likely that your application or a module you’re
using is attempting to use an algorithm or key size which is no longer
allowed by default with OpenSSL 3.0. A new command-line option,
--openssl-legacy-provider, has been added to revert to the legacy
provider as a temporary workaround for these tightened restrictions.

Looks like a webpack issue. This is fixed in webpacl 5+ and we are on
webpack4 at the moment.
Solutions

    Upgrade webpack.
    Pin nodejs version to be 16.x.x
    Use  --openssl-legacy-provider as a workaround.

Pin docker version to alpine3.16 branch to have node16.x by default

ref:
#5555 (comment)
@vishnu-narayanan
Copy link
Member Author

Pinning node version to 16.x for now.

cc: @sojan-official @scmmishra

@vishnu-narayanan
Copy link
Member Author

note: make sure to update the reference to the node version across the repo, like cwctl etc. also ensure that we have a pathway to upgrade node version in those installations

@sojan-official Will take this up as a separate issue. Currently, the node version is different between docker(16.x.x) and linux(12.x.x) installations.

Also, is a node update mandatory for existing installations as part of the chatwoot upgrade?

@sojan-official
Copy link
Member

@vishnu-narayanan would help stay consistent everywhere. There might be differences in support for packages.

@vishnu-narayanan
Copy link
Member Author

CE spec GitHub actions and docker image builds are passing. CircleCI is failing, though. Circle does not have a 3.1.3-browser image with nodev16.x.

Tests are passing locally with node v18.x, which CircleCI is using.

➜  chatwoot git:(feat/upgrade_ruby) node -v
v18.13.0
➜  chatwoot git:(feat/upgrade_ruby) rspec ./spec/controllers/super_admin/installation_configs_controller_spec.rb:34
Run options: include {:locations=>{"./spec/controllers/super_admin/installation_configs_controller_spec.rb"=>[34]}}
.

Finished in 0.83616 seconds (files took 3.48 seconds to load)
1 example, 0 failures

Coverage report generated for RSpec to /Users/vn/Documents/workspace/cw/chatwoot/coverage. 642 / 14130 LOC (4.54%) covered.

cc: @sojan-official

@sojan-official
Copy link
Member

@nithindavid could you take a look at this build ?

@vishnu-narayanan
Copy link
Member Author

@sojan-official Could you please review again?

@vishnu-narayanan vishnu-narayanan merged commit b1ec67d into develop Jan 24, 2023
@vishnu-narayanan vishnu-narayanan deleted the feat/upgrade_ruby branch January 24, 2023 18:25
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sojan-official sojan-official sojan-official approved these changes

Assignees

@sojan-official sojan-official

@vishnu-narayanan vishnu-narayanan

Labels
ready-for-pickup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vishnu-narayanan @sojan-official @pranavrajs @scmmishra