Rails 5-1 #18076

jrafanie · 2018-10-10T18:07:50Z

Remaining items

Extract commits that can land on master for both rails 5.0 and 5.1 (this repo and others)
Fix julian to gregorian time issue
Fix deadlock/database lock test
Figure out what to do with rails 5-1-stable branch (5.1.7 was released)
Deploy to an appliance and look for failures/warnings
Verify debug and compile assets (enabled compile in test / monkey patched sprockets-rails)
Verify haml/haml_lint/erubi change (extracted to Use erubi instead of erubis, upgrade haml/haml_lint #18868)
Write up talk article with the most common rails 5.1 issues DONE, here

Other repos:

Test UI classic repo with this fork as the manageiq repo

For fun, autoload doesn't work in application.rb, so you have to either do it in an config/initializers or require the file. rails/rails@83b767ce Rails issue: #25525 Extracted from ManageIQ#18076

Rails 5.1 removed support for specifying a string/symbol for the middleware class. You must now use a constant. For fun, autoload doesn't work in application.rb, so you have to either do it in an config/initializers or require the file. rails/rails@83b767ce Rails issue: #25525 Extracted from ManageIQ#18076

To avoid relying on the connection to type cast the binds, this method was removed. It was replaced with the underlying implementation of the method, which is also used here to replace our usage. rails/rails@5465508 Extracted from ManageIQ#18076

The force reload parameter was removed from assocations in: rails/rails@09cac8c67af It was previously deprecated but apparently we didn't fix that warning. Extracted from ManageIQ#18076

The force reload parameter was removed from associations in: rails/rails@09cac8c67af It was previously deprecated but apparently we didn't fix that warning. Extracted from ManageIQ#18076

According to 85c0224, we want the cascading delete to avoid orphaned resources, so we'll remove the one that doesn't explicitly say dependent => destroy. Rails 5.1+ complains about has_many through associations if the through association is not yet defined, and in this case, the second host_storages association occurs after the through association, so rails yells: ``` ActiveRecord::HasManyThroughOrderError: Cannot have a has_many :through association 'Storage#hosts' which goes through 'Storage#host_storages' before the through association is defined. ``` Extracted from ManageIQ#18076

Rails 5.1+ complains about has_many through associations if the through association is not yet defined. Extracted from ManageIQ#18076

Rails ~5.1 changed arity from 1 to 0 in commit: rails/rails@a553302 Extracted from ManageIQ#18076

Rails exists? with empty hash in ancestry 2.2.x was broken by rails PR: 28699 Fixed in ancestry 3.0.0: stefankroes/ancestry@8c43482 Extracted from ManageIQ#18076

The force reload parameter was removed from associations in: rails/rails@09cac8c67af It was previously deprecated here: rails/rails@6eae366 From that commit: For collections: @user.posts.reload # Instead of @user.posts(true) For singular associations: @user.reload.profile # Instead of @user.profile(true) Extracted from ManageIQ#18076

Note, the before_save was calling remove_invalid_resource on create which didn't make sense because it was trying to find service_resources with orphaned resources, which can't happen unless the service_template itself has been saved. Because of this, it makes sense to change this to before_update instead of adding r.persisted? checks in the remove_invalid_resource method. The force reload parameter was removed from associations in: rails/rails@09cac8c67af It was previously deprecated here: rails/rails@6eae366 From that commit: For collections: @user.posts.reload # Instead of @user.posts(true) For singular associations: @user.reload.profile # Instead of @user.profile(true) Extracted from ManageIQ#18076

spec/models/vmdb_database_connection_spec.rb

jrafanie · 2019-06-19T14:09:34Z

@himdel @h-kataria @Fryguy @bdunne this is ready to go. The talk article was written. Is there anything else we need?

Here's the talk article.

These can be merged with rails 5.0 (now) as they will go red when 5.1 lands:

The only other remaining items can only be merged after 5.1 is merged since there's no 5.0/5.1 compatible way to fix these (RAILS 5.1 is merged)

Fix attribute_changed? deprecation for rails 5.1 manageiq-providers-ansible_tower#174
Release a connection causes test thread problem - Support Rails 5-1 manageiq-automation_engine#327
Fix 5.1 deprecation on attribute_was, not 5.0 compatible manageiq-providers-openstack#477

bdunne

👍 LGTM

himdel

LGTM, ui-classic is stilll green.

I guess there's a chance we can still hit some params issues (ManageIQ/manageiq-ui-classic#5076), but currently not seeing any 👍

himdel · 2019-06-19T14:50:15Z

Oh, correction, specs are passing locally.
ManageIQ/manageiq-ui-classic#5676 is actually red now, restarting

himdel · 2019-06-19T15:37:13Z

Oh, @jrafanie can you rebase please?
I think the only reason ManageIQ/manageiq-ui-classic#5676 is failing travis is that this branch is based on master before the UI travis fix in #18892

jrafanie · 2019-06-19T16:45:31Z

Oh, @jrafanie can you rebase please?
I think the only reason ManageIQ/manageiq-ui-classic#5676 is failing travis is that this branch is based on master before the UI travis fix in #18892

thanks, I thought I did before court this morning but I rebased the manageiq rails-5-1 branch... ETOOMANYREPOS... I just rebased ui-classi's rails-5-1 test branch... will post here when it is green

jrafanie · 2019-06-19T16:45:53Z

oops, comment is too close to close and comment

Rails 5.1 switched from julian to gregorian calendar so 1.month and 1.year changed in value. Note, duration_of_report_step consumers should be 30.days and sometimes 1.month, ugh rails/rails#27610 (comment) rails/rails@2a5ae2b Rails PR: #29971

See commit: rails/rails@a553302

5.1 added new methods for checking changes, specifically in before callbacks. Summary: 5.0 => 5.1 name_changed? => will_save_change_to_name? (before callback) name_changed? => saved_change_to_name? (after callback) A larger table of changes can be found here: rails/rails#25337 (comment) Use latest ancestry with changes for the changed change: https://github.com/stefankroes/ancestry/pull/441/files

Rails pr #26414

This is basically a revert of e83ca32.

miq-bot · 2019-06-19T17:42:20Z

Checked commits jrafanie/manageiq@c6329a1~...3b7b50d with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
11 files checked, 2 offenses detected

app/models/chargeback.rb

❗ - Line 164, Col 52 - Style/HashSyntax - Use hash rockets syntax.

lib/extensions/ar_migration.rb

❗ - Line 3, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.

jrafanie · 2019-06-19T17:42:33Z

ok, the test rails 5.1 PR on ui classic is now green after rebasing and force pushing this PR.

local developers to bundler up and get rails 5.2. Unfortunately, manageiq isn't yet on rails 5.2 so it can't process 5.2 versioned migrations as seen at least once such as ManageIQ#379. ManageIQ/manageiq#18076 is adding rails 5.1 support to manageiq, so let's change schema to 5.1 for now while we work on 5.2 for manageiq.

manageiq isn't yet on rails 5.2 so it can't process 5.2 versioned migrations as seen at least once already, such as ManageIQ#379. ManageIQ/manageiq#18076 is adding rails 5.1 support to manageiq, so let's change schema to 5.1 for now while we work on 5.2 for manageiq.

PR ManageIQ#365 added support for rails 5.1 and 5.2 to schema. Unfortunately, manageiq isn't yet on rails 5.2 so it can't process 5.2 versioned migrations as seen in ManageIQ#379. ManageIQ/manageiq#18076 is adding rails 5.1 support to manageiq, so let's change schema to 5.1 for now while we work on 5.2 for manageiq.

jrafanie mentioned this pull request Oct 10, 2018

Rails 5.1/5.0 compatibility: Use constant since string/symbol was removed #18077

Merged

jrafanie mentioned this pull request Oct 10, 2018

Rails 5.0/5.1 prepare_binds_for_database is gone #18078

Merged

jrafanie mentioned this pull request Oct 10, 2018

Rails 5.0/5.1 Use o.reload.assoc, assoc(true) is gone #18079

Merged

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Oct 10, 2018

Define the through association before using it

6423d74

Rails 5.1+ complains about has_many through associations if the through association is not yet defined. Extracted from ManageIQ#18076

jrafanie mentioned this pull request Oct 10, 2018

Rails 5.0/5.1 compatibility: Define through association before has many through #18080

Merged

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Oct 10, 2018

Rails 5.0/5.1 Check arity to fix ArgumentErrors

4d26ab6

Rails ~5.1 changed arity from 1 to 0 in commit: rails/rails@a553302 Extracted from ManageIQ#18076

jrafanie mentioned this pull request Oct 10, 2018

Rails 5.0/5.1 Check arity to fix ArgumentErrors #18081

Closed

jrafanie force-pushed the rails-5-1 branch from 0688463 to 424bb86 Compare October 10, 2018 19:30

miq-bot added the wip label Oct 11, 2018

jrafanie force-pushed the rails-5-1 branch from 424bb86 to f93120b Compare October 11, 2018 15:45

jrafanie force-pushed the rails-5-1 branch 2 times, most recently from a5491e4 to 24028ae Compare October 11, 2018 22:01

miq-bot added dependencies unmergeable labels Oct 12, 2018

jrafanie force-pushed the rails-5-1 branch from 24028ae to ff66448 Compare October 15, 2018 19:28

miq-bot removed the unmergeable label Oct 15, 2018

jrafanie force-pushed the rails-5-1 branch from ff66448 to f1ad0f1 Compare October 15, 2018 19:55

jrafanie commented Oct 15, 2018

View reviewed changes

spec/models/vmdb_database_connection_spec.rb Outdated Show resolved Hide resolved

miq-bot removed the wip label Jun 19, 2019

bdunne approved these changes Jun 19, 2019

View reviewed changes

himdel approved these changes Jun 19, 2019

View reviewed changes

jrafanie closed this Jun 19, 2019

jrafanie reopened this Jun 19, 2019

chrisarcand and others added 6 commits June 19, 2019 13:17

Bump to latest Rails 5.1.x

c6329a1

Rails 5.1 changed arity from 1 to 0

eeaa31f

See commit: rails/rails@a553302

Rails commands were refactored

c1bca0a

Rails pr #26414

The deprecation doesn't exist in 5.1

3b7b50d

This is basically a revert of e83ca32.

jrafanie force-pushed the rails-5-1 branch from d1a31a2 to 3b7b50d Compare June 19, 2019 17:18

jrafanie mentioned this pull request Jun 19, 2019

Allow up to 5.1 versioned migrations ManageIQ/manageiq-schema#384

Merged

1 task

bdunne merged commit dbd6f04 into ManageIQ:master Jun 19, 2019

bdunne added this to the Sprint 114 Ending Jun 24, 2019 milestone Jun 19, 2019

bdunne self-assigned this Jun 19, 2019

bdunne added core rails5.1 labels Jun 19, 2019

jrafanie deleted the rails-5-1 branch June 19, 2019 20:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rails 5-1 #18076

Rails 5-1 #18076

jrafanie commented Oct 10, 2018 •

edited

Loading

jrafanie commented Jun 19, 2019 •

edited

Loading

bdunne left a comment

himdel left a comment

himdel commented Jun 19, 2019

himdel commented Jun 19, 2019

jrafanie commented Jun 19, 2019

jrafanie commented Jun 19, 2019

miq-bot commented Jun 19, 2019 •

edited by jrafanie

Loading

jrafanie commented Jun 19, 2019

Rails 5-1 #18076

Rails 5-1 #18076

Conversation

jrafanie commented Oct 10, 2018 • edited Loading

Remaining items

jrafanie commented Jun 19, 2019 • edited Loading

bdunne left a comment

Choose a reason for hiding this comment

himdel left a comment

Choose a reason for hiding this comment

himdel commented Jun 19, 2019

himdel commented Jun 19, 2019

jrafanie commented Jun 19, 2019

jrafanie commented Jun 19, 2019

miq-bot commented Jun 19, 2019 • edited by jrafanie Loading

jrafanie commented Jun 19, 2019

jrafanie commented Oct 10, 2018 •

edited

Loading

jrafanie commented Jun 19, 2019 •

edited

Loading

miq-bot commented Jun 19, 2019 •

edited by jrafanie

Loading