Rails 6.0 #20778
Merged
Rails 6.0 #20778
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NickLaMuro
requested review from
agrare,
Fryguy,
gtanzillo,
jrafanie and
kbrock
as code owners
NickLaMuro
force-pushed
the
rails-6
branch
2 times, most recently
from
845da1c
to
e049dee
Compare
NickLaMuro
force-pushed
the
rails-6
branch
3 times, most recently
from
3cf996f
to
7ca5ca0
Compare
13 tasks
NickLaMuro
force-pushed
the
rails-6
branch
2 times, most recently
from
8710fac
to
bbd1373
Compare
|
@miq-bot remove-label unmergeable |
For Rails 6.0, sqlite3 ~> 1.4 is required:
LoadError:
Error loading the 'sqlite3' Active Record adapter. Missing a gem
it depends on? can't activate sqlite3 (~> 1.4), already activated
sqlite3-1.3.13. Make sure all dependencies are added to Gemfile.
This was changed in rails here:
ManageIQ#20721 (comment)
And is only part of rails 6.0.0.0-rc1 and above.
Update to a version that includes the patch for Rails 6.0 ManageIQ/jquery-rjs#3
Rails 6 changed database configuration to support multi-db environments: rails/rails#33637 So how one accesses the `@config` hash has changed. The original error suggested the following: NotImplementedError: `ActiveRecord::Base.configurations` in Rails 6 now returns an object instead of a hash. The `fetch_path` method is not supported. Please use `configs_for` or consult the documentation for supported methods. However, there is no `[]` or `host` method, so it would require calling `configs_for(env_name: ENV["RAILS_ENV"]).config['host']`. This solution seemed like the more terse solution.
This was changed in new versions of `actionpack`, and without this
change, the following error is given:
TypeError:
superclass mismatch for class TestSession
# actionpack-6.0.3.3/lib/action_controller/test_case.rb:179:in `<module:ActionController>'
# ...
# ./spec/spec_helper.rb:11:in `<top (required)>'
No examples found.
Do to a performance fix in Rails 6: - rails/rails@cc2d614e The Disk#present column would cause the application to raise the following error: ActiveRecord::DangerousAttributeError: present? is defined by Active Record. Check to make sure that you don't have an attribute or method with the same name. This workaround avoids having to create a database migration to fix this column, and instead allows us to keep the column we have had in place for years for a performance fix that doesn't apply to this model anyway.
Do to a performance fix in Rails 6: - rails/rails@cc2d614e The GuestDevice#present column would cause the application to raise the following error: ActiveRecord::DangerousAttributeError: present? is defined by Active Record. Check to make sure that you don't have an attribute or method with the same name. This workaround avoids having to create a database migration to fix this column, and instead allows us to keep the column we have had in place for years for a performance fix that doesn't apply to this model anyway.
Fixes the following deprecation warning in MiqGroup: DEPRECATION WARNING: Class level methods will no longer inherit scoping from `create!` in Rails 6.1. To continue using the scoped relation, pass it into the block directly. To instead access the full set of models, as Rails 6.1 will, use `MiqGroup.default_scoped`. (called from next_sequence at /Users/nicklamuro/code/redhat/manageiq/app/models/miq_group.rb:72) There was a change coming that will prevent leaking scopes in Rails 6.1, and causes a deprecation warning as part of Rails 6.0: - rails/rails#35280 - rails/rails#37727 Since our use case probably benefited from the leaking scope, we needed to support that functionality when it make sense, otherwise default to the .default_scope (aka: self) when appropriate.
Adds a fix for the following deprecation error: DEPRECATION WARNING: Class level methods will no longer inherit scoping from `create!` in Rails 6.1. To continue using the scoped relation, pass it into the block directly. To instead access the full set of models, as Rails 6.1 will, use `MiqGroup.default_scoped`. (called from validate_each at /Users/nicklamuro/code/redhat/manageiq/lib/unique_within_region_validator.rb:25) This was added to fix "leaking scopes" in Rails 6.1, but was added as a deprecation warning first: - rails/rails#35280 - rails/rails#37727
Similar to previous commits, this fixes the following deprecation error that was propagating when running `task_helpers/imports/tags_spec.rb` DEPRECATION WARNING: Class level methods will no longer inherit scoping from `create` in Rails 6.1. To continue using the scoped relation, pass it into the block directly. To instead access the full set of models, as Rails 6.1 will, use `Classification.default_scoped`. (called from validate_each at /Users/nicklamuro/code/redhat/manageiq/app/models/classification.rb:512) This was added to fix "leaking scopes" in Rails 6.1, but was added as a deprecation warning first: - rails/rails#35280 - rails/rails#37727
Adds a fix for the following deprecation error: DEPRECATION WARNING: Class level methods will no longer inherit scoping from `create!` in Rails 6.1. To continue using the scoped relation, pass it into the block directly. To instead access the full set of models, as Rails 6.1 will, use `Vm.default_scoped`. (called from validate_each at /Users/nicklamuro/code/redhat/manageiq/spec/lib/rbac/filterer_spec.rb:2086) This was added to fix "leaking scopes" in Rails 6.1, but was added as a deprecation warning first: - rails/rails#35280 - rails/rails#37727 This specifically was just a fake `Vm` model specifically used for the Rbac specs, so this isn't something that needs to be fixed in the app codebase proper.
In Rails 6, we get the following error:
Failures:
1) MiqRegionRemote.with_remote_connection removes the temporary connection pool
Failure/Error: local_database = ActiveRecord::Base.configurations.fetch_path(Rails.env, "database").to_s.strip
NotImplementedError:
`ActiveRecord::Base.configurations` in Rails 6 now returns an object
instead of a hash. The `fetch_path` method is not supported. Please
use `configs_for` or consult the documentation for supported methods.
# activerecord-6.0.3.4/lib/active_record/database_configurations.rb:221:in `method_missing'
# ./app/models/miq_region_remote.rb:87:in `with_remote_connection'
# ./spec/models/miq_region_remote_spec.rb:24:in `block (3 levels) in <top (required)>'
# webmock-3.9.3/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
This fixes said error, similar to a fix done in `EvmDatabase`.
Some changes to Rails seemed to cause timestamps to be interpreted differently: - rails/rails@57015cd - rails/rails@db077e8 - rails/rails@497e52f - rails/rails@f45b01a In these specs, instead of returning a formatted String, they return Time objects. For our purposes, it is probably better modify the specs instead of changing the mixin to just compare using `Time` objects, which is what was done in this commit. If you are reading this an there is something actually wrong with this approach, you can also change the query being made in the code to return a String timestamp as was previously expected before: diff --git a/tools/radar/rollup_radar_mixin.rb b/tools/radar/rollup_radar_mixin.rb index 905f36cf65..c874b79b49 100644 --- a/tools/radar/rollup_radar_mixin.rb +++ b/tools/radar/rollup_radar_mixin.rb @@ -30,9 +30,8 @@ module RollupRadarMixin end def date_trunc(by, attribute) - Arel::Nodes::NamedFunction.new( - 'DATE_TRUNC', [Arel.sql("'#{by}'"), attribute] - ) + date_truncation = Arel::Nodes::NamedFunction.new('DATE_TRUNC', [Arel.sql("'#{by}'"), attribute]) + Arel::Nodes::NamedFunction.new('TO_CHAR', [date_truncation, Arel.sql("'YYYY-MM-DD HH24:MI:SS'")]) end But this approach seemed to change less, so this was done instead.
This commit in Rails: rails/rails@7cc27d7 Updated the ActiveRecord::MigrationContext.new arguments to also include a second argument that passed the base class(?) for all migrations. Originally, this commit was coded to basically use the default value from the `ActiveRecord::Migration#copy` method: https://github.com/rails/rails/blob/7cc27d749c3563e6b278ad01d233cb92ea3b7935/activerecord/lib/active_record/migration.rb#L887 schema_migration = options[:schema_migration] || ActiveRecord::SchemaMigration Which seemed to cause trouble when running the specs. Using the author of PaperTrail's comment as inspiration: rails/rails#36439 (comment) Making use of ::ActiveRecord::Base.connection.schema_migration seems to work as a good second arg for our use cases.
Seems that a there were a few changes to how the SQL string was generated under the hood as part of the changes from Rails 5.2 to Rails 6, and these changes support that. Extra spacing for the `SELECT 1` case aside, it seems like columns for `INNER JOIN` have been ordered alphabetically now, causing the discrepancy in versions of Rails.
This was changed in Rails 6: rails/rails@e17fe52 And using `.empty` is good enough for our case since we are just trying to access the helper method, `.number_to_human_size`, outside the context of the views.
For some (unknown) reason, this `.as_json` call to this object stringifies all of it's nested keys, instead of the previous implementation in Rails 5.2 where it didn't. Unsure what changed with Rails 5.2 to Rails 6.0, and many hours were wasted trying to determine what did, but since this change hopefully should be minimally impacting, this has just been fixed in the specs.
|
Checked commits NickLaMuro/manageiq@4db0d66~...182afee with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint Gemfile
|
|
Didn't have a good place to put this, but worth noting that we seemingly have had errors like this in our logs for years now: But have only been made more obvious by this commit: Which was introduced in Rails 6. (spent a decent amount of time looking into what controller was causing it, only to find out that it happens in Rails 5.2 as well, just not obvious because of the lack of color in the console output) |
This was referenced Mar 30, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Main pull request for upgrading MIQ to Rails 6
Links