Project

General

Profile

Actions

Idea #14988

closed

[Workbench] Upgrade to Rails 5

Added by Lucas Di Pentima about 5 years ago. Updated almost 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench
Target version:
Story points:
0.5
Release relationship:
Auto

Description

Rails 4.2 is about to be EOLed when 6.0 is released.


Subtasks 1 (0 open1 closed)

Task #15032: Review 14988-wb-rails5-upgradeResolvedLucas Di Pentima05/14/2019Actions

Related issues

Blocks Arvados - Idea #14946: Update to Ruby 2.5 - 2.3 is going EOLResolvedLucas Di Pentima05/31/2019Actions
Actions #1

Updated by Lucas Di Pentima about 5 years ago

  • Blocks Idea #14946: Update to Ruby 2.5 - 2.3 is going EOL added
Actions #2

Updated by Lucas Di Pentima about 5 years ago

  • Target version changed from 2019-03-27 Sprint to 2019-04-10 Sprint
Actions #3

Updated by Tom Morris about 5 years ago

  • Story points set to 3.0
Actions #4

Updated by Lucas Di Pentima about 5 years ago

  • Status changed from New to In Progress
Actions #5

Updated by Lucas Di Pentima about 5 years ago

  • Target version changed from 2019-04-10 Sprint to 2019-04-24 Sprint
Actions #6

Updated by Lucas Di Pentima almost 5 years ago

  • Target version changed from 2019-04-24 Sprint to 2019-05-08 Sprint
Actions #7

Updated by Lucas Di Pentima almost 5 years ago

  • Story points changed from 3.0 to 2.0
Actions #8

Updated by Lucas Di Pentima almost 5 years ago

Status update

Updates at 0ebe4a09a3d12f1415d2c119e716a2788ff889ec - branch 14988-wb-rails5-upgrade

  • I've been eliminating the deprecation warnings after doing the rake app:update call.
  • Forked a couple of gems because they aren't being maintained:
    • activerecord-nulldb-adapter
    • wiselinks: This one is not yet clear that it works, I was able to make it stop complaining about deprecated functions but it's from rails 3.x and it hasn't being updated since then. I've tried to submit a PR to the original maintainer but their travis pipeline fail because it's set up to test against ruby 1.9 & 2.0, and some of the updated dependencies aren't compatible with those ruby environments.
  • I'm now trying to make the unit test pass, but our fake ActiveRecord::Column loading is not working properly, I think in part it's because the ActiveRecord::Column API changed again (we had issues when upgrading from 4.1 to 4.2) and I'm not getting much documentation about how to work with it, so what I'm trying is the new ActiveRecord::Attributes API (https://api.rubyonrails.org/classes/ActiveRecord/Attributes/ClassMethods.html) that seems to be the correct way to go about adding attributes not backed by a database.
Actions #9

Updated by Tom Clegg almost 5 years ago

It's possible that we don't even benefit from wiselinks and removing it would have no effect. Worth trying, if it causes any further trouble.

As for the fake-activerecord stuff, all I can think to offer is that IIRC it has always been a hack, written in "whatever seems to work" style, so it's not too surprising to hear Rails upgrades keep breaking it.

Actions #10

Updated by Lucas Di Pentima almost 5 years ago

  • Target version changed from 2019-05-08 Sprint to 2019-05-22 Sprint
  • Story points changed from 2.0 to 0.5
Actions #11

Updated by Lucas Di Pentima almost 5 years ago

Updates at 28ca19d77 - branch 14988-wb-rails5-upgrade
Test run: https://ci.curoverse.com/job/developer-run-tests/1234/

All tests finally passing. Pending items:

  • Update bootstrap-sass gem to version 3.4.1 to address CVE-2019-8331
  • Fix test runner call to be able to run individual test files.
Actions #13

Updated by Tom Clegg almost 5 years ago

I'm not sure whether anything will care, but this (in arvados_base.rb) seems like it would accidentally match new records:

  def destroyed?
    !(etag || uuid)
  end

This (arvados_base.rb) seems like it silently returns nil for nonexistent columns, and foo['bar'] where bar happens to be a method that raises. It looks like this is replacing Rails.logger.debug "BUG: access non-loaded attribute #{k}" -- did you happen to find out what's relying on this?

  def [](attr_name)
    begin
      send(attr_name)
    rescue
      nil
    end
  end
Actions #14

Updated by Lucas Di Pentima almost 5 years ago

Tom Clegg wrote:

I'm not sure whether anything will care, but this (in arvados_base.rb) seems like it would accidentally match new records:

Fixed, thanks!

This (arvados_base.rb) seems like it silently returns nil for nonexistent columns, and foo['bar'] where bar happens to be a method that raises. It looks like this is replacing Rails.logger.debug "BUG: access non-loaded attribute #{k}" -- did you happen to find out what's relying on this?

Restored the debug message. One of the parts relying on this is apps/workbench/app/views/containers/_show_log.html.erb, not sure why it is calling object[:name] but since this is a rails migration story, I didn't want to go into that rabbit hole :)

Updates at 17cde5d13
Test run: https://ci.curoverse.com/job/developer-run-tests/1241/

Actions #15

Updated by Tom Clegg almost 5 years ago

Thanks, LGTM

Actions #16

Updated by Lucas Di Pentima almost 5 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100
Actions #17

Updated by Tom Morris almost 5 years ago

  • Release set to 15
Actions

Also available in: Atom PDF