manageiq icon indicating copy to clipboard operation
manageiq copied to clipboard
verified publisher icon ManageIQ

Upgrade to Rails 6.1

Open Fryguy opened this issue 4 years ago • 3 comments

(or 7.0 if that's easier)

Rails 6.0 goes EOL Jun 1, 2023

Once Rails 7.0 comes out, 6.0 will go into maintenance mode, and 5.2 will be dropped completely. We need to get ourselves into a position where we are more up to date.

TODO list:

Core:

  • [ ] https://github.com/ManageIQ/manageiq/pull/21652
  • [x] https://github.com/ManageIQ/manageiq/pull/21667
  • [x] https://github.com/ManageIQ/manageiq/pull/21668
  • [x] https://github.com/ManageIQ/manageiq/pull/21669
  • [x] https://github.com/ManageIQ/manageiq/pull/21690
  • [x] https://github.com/ManageIQ/manageiq/pull/21691
  • [x] https://github.com/ManageIQ/manageiq/pull/21692
  • [x] https://github.com/ManageIQ/manageiq/pull/21693
  • [x] https://github.com/ManageIQ/manageiq/pull/21695
  • [x] https://github.com/ManageIQ/manageiq/pull/21745
  • [x] https://github.com/ManageIQ/manageiq/pull/22019

Other repos:

  • [ ] https://github.com/ManageIQ/manageiq-api/pull/1121 (needs to be merged with core PR)
  • [x] https://github.com/ManageIQ/inventory_refresh/pull/98
  • [x] https://github.com/ManageIQ/activerecord-virtual_attributes/pull/97
  • [x] https://github.com/ManageIQ/activerecord-virtual_attributes/pull/101
  • [x] https://github.com/ManageIQ/activerecord-virtual_attributes/pull/103
  • [x] https://github.com/ManageIQ/activerecord-virtual_attributes/pull/105
  • [x] https://github.com/ManageIQ/activerecord-virtual_attributes/pull/106
  • [x] https://github.com/ManageIQ/manageiq-schema/pull/635
  • [x] https://github.com/ManageIQ/inventory_refresh/pull/101
  • [x] https://github.com/ManageIQ/manageiq-providers-azure_stack/pull/77
  • [x] https://github.com/ManageIQ/manageiq-providers-ansible_tower/pull/278
  • [x] https://github.com/ManageIQ/manageiq-providers-ovirt/pull/591
  • [x] https://github.com/ManageIQ/manageiq-providers-ovirt/pull/594
  • [x] https://github.com/ManageIQ/manageiq-ui-classic/pull/8382
  • [x] https://github.com/ManageIQ/manageiq-providers-red_hat_virtualization/pull/7

New Gem Releases:

  • [ ] activerecord-virtual_attributes (needs release 6.1.1 and set gem minimum in core Gemfile)
  • [ ] manageiq-messaging and 1.1.0 released but we need to set rails 6.1 branch to use 1.1.0
  • [x] https://github.com/ManageIQ/activerecord-id_regions/commit/6014d0343e0880f2a49b0707700ddc45dd6f2773 and 0.3.2 released
  • [x] https://github.com/ManageIQ/activerecord-virtual_attributes/commit/4c561be3b565d109ad7e2b098a04de0678f2b56e and 6.1.0 released
  • [x] manageiq-postgres_ha_admin and 3.1.4 released
  • [x] manageiq-api-client and 0.3.6 released the minimum.
  • [x] ovirt metrics and 3.1.0 released
  • [x] https://github.com/ManageIQ/vmware_web_service/commit/06aca229484ac7d0a8fdecf36b0e587d8b2df6f7 and 3.1.0 released
  • [x] inventory_refresh and 1.0.0 released.

Loosen dependencies:

  • [x] https://github.com/ManageIQ/activerecord-id_regions/pull/23
  • [x] https://github.com/ManageIQ/activerecord-id_regions/pull/25
  • [x] https://github.com/ManageIQ/activerecord-virtual_attributes/pull/111
  • [x] https://github.com/ManageIQ/inventory_refresh/pull/103
  • [x] https://github.com/ManageIQ/manageiq-api-client/pull/100
  • [x] https://github.com/ManageIQ/manageiq-api-client/pull/103
  • [x] https://github.com/ManageIQ/manageiq-messaging/pull/66
  • [x] https://github.com/ManageIQ/manageiq-messaging/pull/68
  • [x] https://github.com/ManageIQ/manageiq-postgres_ha_admin/pull/28
  • [x] https://github.com/ManageIQ/manageiq-postgres_ha_admin/pull/30
  • [x] https://github.com/ManageIQ/manageiq-providers-ovirt/pull/591
  • [x] https://github.com/ManageIQ/manageiq-ui-classic/pull/8086
  • [x] https://github.com/ManageIQ/ovirt_metrics/pull/41
  • [x] https://github.com/ManageIQ/ovirt_metrics/pull/42
  • [x] https://github.com/ManageIQ/vmware_web_service/pull/113

Note, it doesn't look like we can use bundler plugins within appraisal which were are using for testing multiple rails gem versions. Will need to look at the remaining PRs to figure this out. We use the plugin method for bundler-inject. https://github.com/thoughtbot/appraisal See issue 178

Fryguy avatar Oct 25 '21 17:10 Fryguy

cc @kbrock @jrafanie

Fryguy avatar Oct 25 '21 17:10 Fryguy

Upgrading virtual attributes will be a very big effort. Our main hook, arel_attribute has been removed and instead rails is using Arel::Table[:column]

I have tried a few times to go closer to the attributes api, but the internals of rails keeps the attributes and the columns very separate. So we need to find a way to have our custom attributes defined as columns.

Finding a way to get them into common table expressions feels like the right way forward, though I'm not totally sure of the implications.

Also, rails has a few bugs that make our job of having efficient queries hard to do:

  • includes assumes that you want every field from another table, and there isn't a way to use select() to limit columns on a secondary object.
  • select() assumes you want to specify every column that is part of SELECT rather than saying that you would like additional columns. Matthewd did say that he wanted a fix for this.

I keep thinking that we want some way of specifying a SELECT and a WHERE so it will figure out the joins for us. Also feels like we need to start doing a better job distinguishing between something in the WHERE/ORDER BY clause, and something that needs to be available once we get to ruby. (rails calls them eager load vs preload) summary

So as you can see, I'm conflating a number of issues and that is getting in my way of solving the simple question of getting virtual attributes up to par for 6.1, let alone 7.0

kbrock avatar Oct 26 '21 17:10 kbrock

update to ^ comment. virtual attributes is handles

kbrock avatar Feb 02 '22 21:02 kbrock

Game plan for merging rails 6.1:

  • [x] wait for green cross repo: https://github.com/ManageIQ/manageiq-cross_repo-tests/pull/701 (2 known and expected failures)
  • [x] merge https://github.com/ManageIQ/manageiq/pull/21652
  • [x] merge https://github.com/ManageIQ/manageiq-api/pull/1121

jrafanie avatar Aug 10 '22 19:08 jrafanie

Ok, 2 failures on cross repo. UI Service is known to fail. ManageIQ-api is failing with sporadic failures we've only seen on cross repo tests and also exist with rails 6.0.

I've tried various tests locally and it all worked with manageiq-api and rails 6.1:

api PR with rails 6.1 core repo symlinked:

Finished in 8 minutes 55 seconds (files took 6.2 seconds to load)
3298 examples, 0 failures

Randomized with seed 22019

Cross repo with the same seed:

SPEC_OPTS="--seed 22019" CI=true REPOS="ManageIQ/manageiq#21652,ManageIQ/manageiq-api#1121" TEST_REPO="ManageIQ/manageiq-api#1121" bundle exec manageiq-cross_repo
...
Finished in 10 minutes 15 seconds (files took 8.63 seconds to load)
3298 examples, 0 failures

Randomized with seed 22019

jrafanie avatar Aug 11 '22 14:08 jrafanie

Great work @jrafanie and @kbrock !! 🎉

Fryguy avatar Aug 11 '22 15:08 Fryguy

Just wanted to note here that if we need to revert, we only need to revert:

  • https://github.com/ManageIQ/manageiq/pull/21652
  • https://github.com/ManageIQ/manageiq-api/pull/1121
  • https://github.com/ManageIQ/manageiq-schema/pull/659
  • https://github.com/ManageIQ/manageiq-ui-classic/pull/8411
  • https://github.com/ManageIQ/manageiq-appliance_console/pull/186
  • https://github.com/ManageIQ/manageiq/pull/22053

Fryguy avatar Aug 11 '22 15:08 Fryguy