Rails 7 #22873

jrafanie · 2024-02-01T22:48:53Z

Part of #22052

Cross repo test

After merging this, please merge the following:

jrafanie · 2024-07-31T16:14:01Z

lib/extensions/descendant_loader.rb

+
+  def excluded_parent?(parent)
+    EXCLUDED_PARENTS.include?(parent.to_s)
+  end


Ok, I've split this excluded parents concept into it's own commit. We can choose to drop it as it's only an optimization. We can followup with it if it's questionable.

This isn't needed. @kbrock helped me find out that if the __update_callbacks entrypoint for descendants is excluded, there is no time that we are calling descendants on these high level classes. Dropping this commit.

Fryguy · 2024-07-31T21:50:13Z

spec/lib/miq_preloader_spec.rb


      expect { preload(emses, :vms, vms) }.not_to make_database_queries
      expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries
-      expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries
+
+      # This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor


Very minor, but can you mark this with a TODO tag?

Suggested change

# This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor

# TODO: This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor

Perhaps the other Rails 7 things with the database query counts should also be marked as TODO ?

Thanks, that's a good idea. Keenan and I were discussing it yesterday and they're the only thing we've left to look at in this PR. We're trying a quick dive into them to see if we can determine why they need to change and we'll either mark them TODO as you suggest, pending (for failed assertions that we previously thought should still work... see line 81 below), or some combination.

Fryguy · 2024-07-31T21:50:57Z

spec/lib/miq_preloader_spec.rb


      expect { preload(emses, :vms, vms) }.not_to make_database_queries
      expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries
-      expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries
+
+      # This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor


Here too

Suggested change

# This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor

# TODO: This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor

jrafanie · 2024-08-01T13:59:50Z

Gemfile

@@ -69,8 +69,8 @@ gem "psych",                            ">=3.1",             :require => false #
 gem "query_relation",                   "~>0.1.0",           :require => false
 gem "rack",                             ">=2.2.6.4",         :require => false
 gem "rack-attack",                      "~>6.5.0",           :require => false
-gem "rails",                            "~>6.1.7", ">=6.1.7.8"
-gem "rails-i18n",                       "~>6.x"
+gem "rails",                            "~>7.0.8", ">=7.0.8.4"


Just pushed this change to set minimum to ensure latest CVE coverage

Require minimum for the CVE fixes: https://rubyonrails.org/2024/6/4/Rails-Versions-6-1-7-8-7-0-8-4-and-7-1-3-4-have-been-released Use virtual attributes 7.0 and other gems that support rails 7.

See rails commit: e3b9779cb701c63012bc1af007c71dc5a888d35a Very similar to the change found in: https://github.com/ManageIQ/activerecord-virtual_attributes/pull/133/files Stub/mock the preloader in a rails 7 compatible way Rails 7 preloader requires a scope relation, not an array Note, it looks like only singular associations for already available_records are preloaded with an optimization in rails 7. See: https://www.github.com/rails/rails/pull/42654

Fixes the following deprecation warnings: DEPRECATION WARNING: Time#to_s(:db) is deprecated. Please use Time#to_fs(:db) instead. (called from block (2 levels) in add_metric_rollups_for at /Users/joerafaniello/Code/manageiq/spec/support/chargeback_helper.rb:43) DEPRECATION WARNING: Integer#to_s(:human_size) is deprecated. Please use Integer#to_fs(:human_size) instead. (called from block in build_disk_message at /Users/joerafaniello/Code/manageiq/app/models/vm_reconfigure_task.rb:43) DEPRECATION WARNING: Integer#to_s(:human_size) is deprecated. Please use Integer#to_fs(:human_size) instead. (called from block (3 levels) in <main> at /Users/joerafaniello/Code/manageiq/spec/models/vm_reconfigure_task_spec.rb:45) DEPRECATION WARNING: Integer#to_s(:human_size) is deprecated. Please use Integer#to_fs(:human_size) instead. (called from block (3 levels) in <main> at /Users/joerafaniello/Code/manageiq/spec/models/vm_reconfigure_task_spec.rb:58) DEPRECATION WARNING: Integer#to_s(:human_size) is deprecated. Please use Integer#to_fs(:human_size) instead. (called from block (3 levels) in <main> at /Users/joerafaniello/Code/manageiq/spec/models/vm_reconfigure_task_spec.rb:59)

Rails changed in 7.0 to call subclasses from reload_schema_from_cache here: rails/rails@6f30cc0 It also changed to call descendants on the callback class (self) insead of the ActiveSupport::DescendantsTracker here: rails/rails@ffae3bd We're now getting called in descendants and subclasses from rails very early, at code load time, before models or as models are in between loading. We cannot trigger loads of subclasses while we're loading the existing class so we must wait and avoid it at this point.

jrafanie · 2024-08-06T13:42:33Z

lib/extensions/descendant_loader.rb

    def descendants
-      if Vmdb::Application.instance.initialized? && !defined? @loaded_descendants
+      if Vmdb::Application.instance.initialized? && !defined?(@loaded_descendants) && %w[__update_callbacks].exclude?(caller_locations(1..1).first.base_label)


the bot/cop complained about using caller_locations to build the full array so I accepted that suggestion even though I think it makes it a bit more convoluted. This method can be run many times, especially when models aren't yet loaded, so it should be as performant as possible.

miq-bot · 2024-08-06T13:45:45Z

Checked commits jrafanie/manageiq@3b74f5a~...b85a0c7 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
10 files checked, 2 offenses detected

app/models/vm_reconfigure_task.rb

❗ - Line 43, Col 53 - Style/StringConcatenation - Prefer string interpolation to string concatenation.

spec/models/vm_reconfigure_task_spec.rb

❗ - Line 59, Col 7 - Layout/LineEndStringConcatenationIndentation - Indent the first part of a string concatenated with backslash.

GilbertCherrie

Ui seems to be working correctly and updates are being loaded correctly as well.

Fixes the following error on rails 7.1: TypeError: no implicit conversion of Symbol into Integer Shared Example Group: "used" called from ./spec/content/automate/ManageIQ/System/CommonMethods/QuotaMethods.class/__methods__/used_spec.rb:59 ./content/automate/ManageIQ/System/CommonMethods/QuotaMethods.class/__methods__/used.rb:25:in `to_s' ./content/automate/ManageIQ/System/CommonMethods/QuotaMethods.class/__methods__/used.rb:25:in `used' ./content/automate/ManageIQ/System/CommonMethods/QuotaMethods.class/__methods__/used.rb:18:in `main' ./spec/content/automate/ManageIQ/System/CommonMethods/QuotaMethods.class/__methods__/used_spec.rb:41:in `block (3 levels) in <top (required)>' Fixes the following deprecation warnings that should have picked this up on rails 7: DEPRECATION WARNING: Time#to_s(:db) is deprecated. Please use Time#to_fs(:db) instead. DEPRECATION WARNING: Integer#to_s(:human_size) is deprecated. Please use Integer#to_fs(:human_size) instead. A similar change was fixed in here in commit: ManageIQ/manageiq@21bc1a9 Part of this PR for Rails 7: ManageIQ/manageiq#22873

jrafanie added enhancement rails7 labels Feb 1, 2024

jrafanie requested review from agrare, kbrock, jeffibm and Fryguy as code owners February 1, 2024 22:48

jrafanie mentioned this pull request Feb 1, 2024

Upgrade to rails 7.0 #22052

Closed

miq-bot added wip core/embedded ansible dependencies labels Feb 1, 2024

jrafanie force-pushed the rails7 branch 4 times, most recently from a495d2d to 59d87c0 Compare February 5, 2024 22:59

jrafanie force-pushed the rails7 branch from 59d87c0 to 9e0020a Compare February 7, 2024 17:22

miq-bot added the unmergeable label Feb 7, 2024

jrafanie force-pushed the rails7 branch 4 times, most recently from 322ef1c to 803369e Compare February 9, 2024 19:18

miq-bot removed the unmergeable label Feb 10, 2024

miq-bot added the unmergeable label Feb 21, 2024

jrafanie force-pushed the rails7 branch from 803369e to 4a69a73 Compare February 21, 2024 20:31

jrafanie mentioned this pull request Feb 21, 2024

Only run load_subclasses after app is initialized #22904

Merged

jrafanie force-pushed the rails7 branch from 4a69a73 to 3447c7a Compare February 21, 2024 21:46

miq-bot removed the unmergeable label Feb 21, 2024

jrafanie force-pushed the rails7 branch from 2394fcd to bc3564b Compare July 31, 2024 16:11

jrafanie commented Jul 31, 2024

View reviewed changes

jrafanie force-pushed the rails7 branch 5 times, most recently from c17092f to 0fda41e Compare July 31, 2024 21:14

Fryguy reviewed Jul 31, 2024

View reviewed changes

jrafanie force-pushed the rails7 branch from 0fda41e to e8b7694 Compare August 1, 2024 13:59

jrafanie commented Aug 1, 2024

View reviewed changes

jrafanie force-pushed the rails7 branch from e8b7694 to cba105f Compare August 1, 2024 22:27

jrafanie mentioned this pull request Aug 2, 2024

Rails7 test fixes ManageIQ/manageiq-ui-classic#9231

Merged

2 tasks

jrafanie changed the title ~~[WIP] Rails7~~ Rails 7 Aug 2, 2024

jrafanie removed the wip label Aug 2, 2024

jrafanie added 5 commits August 6, 2024 09:40

Update to latest rails 7.0 gems

3b74f5a

Require minimum for the CVE fixes: https://rubyonrails.org/2024/6/4/Rails-Versions-6-1-7-8-7-0-8-4-and-7-1-3-4-have-been-released Use virtual attributes 7.0 and other gems that support rails 7.

Drop no longer needed setting in rails 7

9b6ebb1

jrafanie force-pushed the rails7 branch from cba105f to b85a0c7 Compare August 6, 2024 13:40

jrafanie commented Aug 6, 2024

View reviewed changes

Fryguy self-assigned this Aug 6, 2024

Fryguy approved these changes Aug 6, 2024

View reviewed changes

GilbertCherrie approved these changes Aug 6, 2024

View reviewed changes

Fryguy merged commit 6f8fdeb into ManageIQ:master Aug 6, 2024
8 checks passed

jrafanie deleted the rails7 branch August 9, 2024 20:43

jrafanie mentioned this pull request Jan 30, 2025

Fix rails 7 deprecation/7.1 removal by using Integer#to_fs(:human_size) ManageIQ/manageiq-content#759

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rails 7 #22873

Rails 7 #22873

jrafanie commented Feb 1, 2024 •

edited by Fryguy

Loading

jrafanie Jul 31, 2024

jrafanie Jul 31, 2024

Fryguy Jul 31, 2024

Fryguy Jul 31, 2024

jrafanie Aug 1, 2024

Fryguy Jul 31, 2024

jrafanie Aug 1, 2024

jrafanie Aug 6, 2024

miq-bot commented Aug 6, 2024

GilbertCherrie left a comment

	# This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor
	# TODO: This PR seems to indicate only the singular associations are preloaded after the rails 7 refactor

Rails 7 #22873

Rails 7 #22873

Conversation

jrafanie commented Feb 1, 2024 • edited by Fryguy Loading

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

miq-bot commented Aug 6, 2024

GilbertCherrie left a comment

Choose a reason for hiding this comment

jrafanie commented Feb 1, 2024 •

edited by Fryguy

Loading