Upgrade to Rails 6.1 / Ruby 2.7 #663

Fryguy · 2023-11-21T14:12:20Z

This makes a lot more sense if we go commit by commit.

I can pull some of these out as separate PRs to make this easier.

@bdunne @jrafanie I'd like your thoughts on how to move forward.

I tried to get to Ruby 3, but the hard part is actually upgrading faraday, because the specs mess around with the guts of faraday which have changed.

Config was taken from a vanilla Rails 6.1

Fryguy · 2023-11-21T14:18:40Z

.gitignore

@@ -39,5 +39,4 @@
 # Ignore local seeds file
 db/seeds.local.rb

-# Ignore vagrant configuration/runtime files
-.vagrant/
+/config/master.key


This is something I think we need to put in as a secret in kube that maps to this location, and I guess keep in our vault separately.

Fryguy · 2023-11-21T14:19:23Z

Gemfile

-gem 'sync',                              :require => false # required by more_core_extensions
-gem 'rugged',                            :require => false
+gem 'minigit', '~> 0.0.4'
+gem 'net-ssh', '~> 7.2.0'


We can probably drop the ~> here or use >=

wow, that's a version bump. 😉

jrafanie

A few comments. Looks good so far.

jrafanie · 2023-11-21T15:51:42Z

Gemfile

-gem 'sync',                              :require => false # required by more_core_extensions
-gem 'rugged',                            :require => false
+gem 'minigit', '~> 0.0.4'
+gem 'net-ssh', '~> 7.2.0'


wow, that's a version bump. 😉

jrafanie · 2023-11-21T15:54:26Z

app/workers/pull_request_monitor_handlers/merge_target_titler.rb

-  include Sidekiq::Worker
-  sidekiq_options :queue => :miq_bot
+module PullRequestMonitorHandlers
+  class MergeTargetTitler


oh ruby and collapsed namespacing random bugs it causes

I actually don't think this was related. This was because of that eager load thing. Even so, I want to keep this, since it was the only outlier.

Yes, it might not have been the cause this time but it certainly can cause weird constant lookup strangeness.

jrafanie · 2023-11-21T15:55:39Z

Vagrantfile

-# -*- mode: ruby -*-
-# vi: set ft=ruby :
-
-Vagrant.configure("2") do |config|


Were systemd and vagrant setups we wanted to resurrect later? Maybe systemd?

No I don't think so, but git is forever :)

jrafanie · 2023-11-21T15:58:54Z

bin/setup

+
+  puts "\n== Restarting application server =="
+  system! 'bin/rails restart'
+end


Curiosity. I don't see a commit title related to this change, is this just normalizing bin/setup to what we've done elsewhere or is it solving a problem you hit in the upgrade?

Normalizing to the default bin/setup.

Actually most of this is the default Rails file - only the part about database.yml.sample was normalizing.

cool, it wasn't clear if you just took the defaults and changed them after or left them as is. 👍

jrafanie · 2023-11-21T16:04:18Z

config/initializers/filter_parameter_logging.rb

@@ -1,4 +1,6 @@
 # Be sure to restart your server when you modify this file.

 # Configure sensitive parameters which will be filtered from the log file.
-Rails.application.config.filter_parameters += [:password]
+Rails.application.config.filter_parameters += [
+  :passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn


is it intentional to drop :password from this list? Not sure if this is rails defaults overriding our values.

filter parameters are substring match, so passw includes password. This is the same original file, so we didn't "add" password originally as it was the default.

jrafanie · 2023-11-21T16:07:07Z

Can you update the CI ruby to test against 2.7?

Fryguy · 2023-11-21T16:15:33Z

config/environments/development.rb

-  config.eager_load = true
+  config.eager_load = false


@jrafanie 😭

Gemfile.lock

Fryguy · 2023-11-21T16:49:47Z

Can you update the CI ruby to test against 2.7?

Done.

Fryguy · 2023-11-21T16:57:57Z

Tests are failing because rails db:prepare doesn't seem to be doing what it's supposed to do on CI - I'll dig in.

Fryguy · 2023-11-21T18:01:16Z

bin/setup

+  if ENV["CI"]
+    system! 'RAILS_ENV=test bin/rails db:prepare'
+  else


I'm not sure why we need this - feels like a bug in Rails, but it's fine for now.

jrafanie · 2023-11-21T18:41:20Z

Bump tp Ruby 2.7 for GHA

noticed this typo in the commit message

jrafanie · 2023-11-21T18:42:33Z

Tests are failing because rails db:prepare doesn't seem to be doing what it's supposed to do on CI - I'll dig in.

Hmmm, is it possible it's trying to load from schema.rb?

miq-bot · 2023-11-22T19:15:16Z

Checked commits Fryguy/miq_bot@2b0cea1~...68b775b with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
38 files checked, 16 offenses detected

**

💣 💥 🔥 🚒 - Linter/Yaml - missing config files

config/application.rb

❗ - Line 12, Col 5 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.

config/environments/development.rb

❗ - Line 20, Col 6 - Rails/FilePath - Prefer Rails.root.join('path/to').

config/environments/production.rb

❗ - Line 56, Col 22 - Layout/SpaceInsideArrayLiteralBrackets - Do not use space inside array brackets.
❗ - Line 56, Col 34 - Layout/SpaceInsideArrayLiteralBrackets - Do not use space inside array brackets.
❗ - Line 92, Col 50 - Style/GlobalStdStream - Use $stdout instead of STDOUT.

config/initializers/session_store.rb

❗ - Line 3, Col 1 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.
❗ - Line 3, Col 55 - Style/HashSyntax - Use hash rockets syntax.

config/initializers/wrap_parameters.rb

❗ - Line 8, Col 19 - Style/HashSyntax - Use hash rockets syntax.

config/puma.rb

❗ - Line 18, Col 10 - Style/RedundantFetchBlock - Use fetch("PORT", 3000) instead of fetch("PORT") { 3000 }.
❗ - Line 7, Col 25 - Style/RedundantFetchBlock - Use fetch("RAILS_MAX_THREADS", 5) instead of fetch("RAILS_MAX_THREADS") { 5 }.

lib/git_service/branch.rb

❗ - Line 107, Col 27 - Style/OptionalBooleanParameter - Prefer keyword arguments for arguments with a boolean default value; use merged: false instead of merged = false.
❗ - Line 11, Col 38 - Style/OptionalBooleanParameter - Prefer keyword arguments for arguments with a boolean default value; use merged: false instead of merged = false.
❗ - Line 96, Col 26 - Style/OptionalBooleanParameter - Prefer keyword arguments for arguments with a boolean default value; use merged: false instead of merged = false.

spec/models/batch_job_spec.rb

❗ - Line 107, Col 9 - Layout/FirstArrayElementIndentation - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.
❗ - Line 146, Col 11 - Layout/FirstArrayElementIndentation - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.

Fryguy added 15 commits November 17, 2023 08:58

Upgrade to Rails 6.0 / Ruby 2.7 via rake app:update

2b0cea1

Fix issues where Rails 6 requires belongs_to to exist

40676b7

Fix Ruby 2.7 issue where module is not being recognized

98007e5

Fix issue where lib file is not loaded because the const already exists

a03ec03

Eager-load in development

6e304ec

Switch to Rails 6 default configuration

9e7780b

Switch to use zeitwerk as the default

11bae38

Replace thin with puma

bc62a50

Config was taken from a vanilla Rails 6.1

Upgrade to Rails 6.1

627d774

Update a lot of gems

6fc8329

Fix issue where running specs infects Redis

c1fc65e

Update more gems

8240130

Drop systemd

85c08e6

Drop Vagrant

7b8fc27

Update to Ruby 2.7 and NodeJs 18 in Dockerfile

8dc1397

miq-bot added the wip label Nov 21, 2023

Fryguy assigned bdunne and jrafanie Nov 21, 2023

Fryguy added enhancement dependencies Pull requests that update a dependency file labels Nov 21, 2023

Fryguy commented Nov 21, 2023

View reviewed changes

jrafanie reviewed Nov 21, 2023

View reviewed changes

Fryguy commented Nov 21, 2023

View reviewed changes

Gemfile.lock Show resolved Hide resolved

Add ruby and x86_64-linux to the lockfile

3348f61

jrafanie reviewed Nov 21, 2023

View reviewed changes

Gemfile.lock Show resolved Hide resolved

Bump tp Ruby 2.7 for GHA

ea4c1ca

Fix issue where bin/setup was not creating the test DB in CI

d8928ea

Fryguy commented Nov 21, 2023

View reviewed changes

Fryguy added 5 commits November 22, 2023 14:08

Ensure manageiq-style 1.4.0

38bb51c

Fix issue where ActiveRecord::RecordInvalid was not used correctly

1e98c2e

Fix issue where rules settings may not exist

b5306ef

Fix issue where extract_file couldn't handle symlinks

7d8c321

Fix issue where haml-lint executable is different

68b775b

Fryguy changed the title ~~[WIP] Upgrade to Rails 6.1 / Ruby 2.7~~ Upgrade to Rails 6.1 / Ruby 2.7 Nov 22, 2023

miq-bot removed the wip label Nov 22, 2023

bdunne merged commit 3e28293 into ManageIQ:master Nov 22, 2023
2 checks passed

Fryguy deleted the rails6_ruby27 branch November 22, 2023 20:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Rails 6.1 / Ruby 2.7 #663

Upgrade to Rails 6.1 / Ruby 2.7 #663

Fryguy commented Nov 21, 2023 •

edited

Loading

Fryguy Nov 21, 2023

Fryguy Nov 21, 2023 •

edited

Loading

jrafanie Nov 21, 2023

jrafanie left a comment

jrafanie Nov 21, 2023

jrafanie Nov 21, 2023

Fryguy Nov 21, 2023

jrafanie Nov 21, 2023

jrafanie Nov 21, 2023

Fryguy Nov 21, 2023

jrafanie Nov 21, 2023

Fryguy Nov 21, 2023

Fryguy Nov 21, 2023

jrafanie Nov 21, 2023

jrafanie Nov 21, 2023

Fryguy Nov 21, 2023

jrafanie commented Nov 21, 2023

Fryguy Nov 21, 2023

Fryguy commented Nov 21, 2023

Fryguy commented Nov 21, 2023

Fryguy Nov 21, 2023

jrafanie commented Nov 21, 2023

jrafanie commented Nov 21, 2023 •

edited

Loading

miq-bot commented Nov 22, 2023

Upgrade to Rails 6.1 / Ruby 2.7 #663

Upgrade to Rails 6.1 / Ruby 2.7 #663

Conversation

Fryguy commented Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Fryguy Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrafanie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrafanie commented Nov 21, 2023

Choose a reason for hiding this comment

Fryguy commented Nov 21, 2023

Fryguy commented Nov 21, 2023

Choose a reason for hiding this comment

jrafanie commented Nov 21, 2023

jrafanie commented Nov 21, 2023 • edited Loading

miq-bot commented Nov 22, 2023

Fryguy commented Nov 21, 2023 •

edited

Loading

Fryguy Nov 21, 2023 •

edited

Loading

jrafanie commented Nov 21, 2023 •

edited

Loading