Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 6 #516

Merged
merged 23 commits into from
Jan 4, 2021
Merged

Rails 6 #516

merged 23 commits into from
Jan 4, 2021

Conversation

markets
Copy link
Collaborator

@markets markets commented Sep 25, 2019

Rails 6 🎉

Extras:

Notes:

  • Arguments changes in tests has been fixed automatically by the Rails/HttpPositionalArguments cop
  • Coffee to JS, by https://decaffeinate-project.org + minor tweaks/refactor (loadTags function to DRY code)
  • Fixed all potential security vulnerabilities in deps

@markets markets added the wip label Sep 25, 2019
@markets
Copy link
Collaborator Author

markets commented Sep 25, 2019

Current status:

  • App boots fine
  • Most of parts I tested/played work ~well
  • Tests still not reviewed
  • config/environments/* pending to restore custom settings (if any)

@markets markets mentioned this pull request Feb 27, 2020
@markets markets self-assigned this Feb 27, 2020
@sauloperez
Copy link
Collaborator

@markets wouldn't we a bit less risky to do a major revamp of all our gems in our current version and then going through a more progressive migration path, from 4.2 to 5, then to 5.2 and finally to 6? again, I'm just aiming to minimize anything that requires our attention.

@markets
Copy link
Collaborator Author

markets commented Mar 10, 2020

Hi @sauloperez 👋

I don't think so, this way we need to test the app only 1 time. Locally (before the fist commit I mean), I already made the transition from 4.2 -> 5.0 -> 5.2 -> 6.0. I know this is really big branch 😺, but I prefer 1 big branch and test the whole thing 1 time vs several Rails upgrades and tests.

On the other hand, during the latest 4/5 months, I worked on this topic in my company (we upgraded 5 BIG apps from 4.2 -> 6.0), so more or less I have all the "tricks" and common steps fresh in my mind :))

I'll finalize this after the Elastic ✂️ (#551), so I don't need to migrate the Elastic gems. I really think we can do it this way and test the app only once (ok should be a depth test 💪, but only 1 time) I'll test with care locally before removing the Draft badge and pass it officially to ready for review.

@sauloperez
Copy link
Collaborator

Ok, @markets far enough. I give you all my trust 😂 . By the way, those migration skills you got might come in handy in another project you might know about 😏

@markets
Copy link
Collaborator Author

markets commented Apr 3, 2020

Pending:

  • Review specs
  • config/environments/* restore custom settings (if any)

@markets
Copy link
Collaborator Author

markets commented Apr 4, 2020

Update: only specs still pending to be reviewed. I just made a first attempt, and I got a lot of errors from ElasticSearch:

Elasticsearch::Transport::Transport::Errors::BadRequest: [400] {"error":{"root_cause":[{"type":"invalid_type_name_exception","reason":"Document mapping type name can't start with '_', found: [doc]"}],"type":"invalid_type_name_exception","reason":"Document mapping type name can't start with '', found: [_doc]"},"status":400}

But as pointed above, I'll prefer to avoid this migration, so I'll wait until the ElasticSearch ✂️.

A deploy fails because the loaded version and the one specified in
config/deploy.rb don't match.
@sauloperez
Copy link
Collaborator

sauloperez commented Dec 18, 2020

I tried to deploy it to staging but we need more changes to provisioning. Capistrano raises the following:

bundle stderr: /home/timeoverflow/.rbenv/versions/2.6.3/lib/ruby/2.6.0/rubygems.rb:283:in `find_spec_for_exe': Could not find 'bundler' (2.1.4) required by your /var/www/timeoverflow/releases/20201218080546/Gemfile.lock. (Gem::GemNotFoundException)
To update to the latest version installed on your system, run `bundle update --bundler`.
To install the missing version, run `gem install bundler:2.1.4`                                                                 
        from /home/timeoverflow/.rbenv/versions/2.6.3/lib/ruby/2.6.0/rubygems.rb:302:in `activate_bin_path'
        from /home/timeoverflow/.rbenv/versions/2.6.3/bin/bundle:23:in `<main>' 

We need to upgrade bundler in both servers, which I'm doing in coopdevs/timeoverflow-provisioning#183.

sauloperez added a commit to coopdevs/timeoverflow-provisioning that referenced this pull request Dec 18, 2020
This is the version the Gemfile.lock at
coopdevs/timeoverflow#516 defines.
sauloperez added a commit to coopdevs/timeoverflow-provisioning that referenced this pull request Dec 18, 2020
This only install the required bundler version when installing a new
Ruby version. It does not do that when Ruby is unchanged. To solve this
I did a `gem install bundler -v 2.1.4` manually. All pior efforts using
the command module failed so I figured it wasn't a big deal since it's
a change that happens very rarely.

This is bundler version is the one Gemfile.lock at
coopdevs/timeoverflow#516 defines.
sauloperez added a commit to coopdevs/timeoverflow-provisioning that referenced this pull request Dec 18, 2020
This only installs the required bundler version when installing a new
Ruby version. It does not do that when Ruby is unchanged. To solve this
I did a `gem install bundler -v 2.1.4` manually. All prior efforts using
the command module failed so I figured it wasn't a big deal since it's
a change that happens very rarely.

This is bundler version is the one Gemfile.lock at
coopdevs/timeoverflow#516 defines.
@sauloperez
Copy link
Collaborator

Now we got passed bundler but there's another failure.

05:32 deploy:assets:precompile                                                                                                                                                                                                                                  
      01 $HOME/.rbenv/bin/rbenv exec bundle exec rake assets:precompile                                                                                                                                                                                         
      01 rake aborted!                                                                                                                                                                                                                                          
      01 Uglifier::Error: Unexpected token: operator (>). To use ES6 syntax, harmony mode must be enabled with Uglifier.new(:harmony => true).                       
      01 --                                                                                                                                                                                                                                                     
      01  101     ajax: {                                                                                                                                                                                                                                       
      01  102       url: '/tags.json',                                                                                                                                                                                                                          
      01  103       data: function(params) {                                                                                                                                                                                                                    
      01  104         return { term: params.term };                                                                                                                                                                                                             
      01  105       },
      01  106       processResults: function(data, params) {
      01  107         // parse the data into the format expected by Select2
      01  108         return {
      01   =>           results: $.map(data, item => ({
      01  110             id: item,
      01  111             text: item
      01  112           }))
      01  113         };
      01  114       }
      01  115     }
      01  116   });
      01  117 });

you should be able to precompile assets locally and debug it @markets. We should also give you permission to deploy to speed things up.

@markets
Copy link
Collaborator Author

markets commented Dec 18, 2020

Cool @sauloperez! I'll fix that one about js compilation, thanks!

@markets
Copy link
Collaborator Author

markets commented Dec 18, 2020

Fixed by 88b898f ✔️

@sseerrggii
Copy link
Contributor

Thanks @markets @sauloperez for these great efforts 👏 👏 👏

@sauloperez
Copy link
Collaborator

Wow! all tests passing! ✔️ 🏁

@sauloperez
Copy link
Collaborator

All yours for testing @sseerrggii . I just deployed it to staging.

@sseerrggii
Copy link
Contributor

👍

@markets markets removed the wip label Dec 18, 2020
@sseerrggii
Copy link
Contributor

sseerrggii commented Dec 21, 2020

Tested!! Rails 6 seems to work perfect, maybe it's my imagination but the app flies 🛩️

I needed to edit config/environments/development.rb and add config.hosts = "timeoverflow.local" in order to use our development environment. I'm not sure if I should commit that?

I noticed one problem, but I'm not sure if its Rails 6 related, I think it happened to me sometimes in the past 🤔 but on master works fine.... On /statistics_all_transfers i get:

Screenshot_2020-12-21 Action Controller Exception caught(1)

@sseerrggii
Copy link
Contributor

Si! Perfecte! Thanks @markets for me it's green @sauloperez

✔️

@markets
Copy link
Collaborator Author

markets commented Dec 21, 2020

👍🏼 @sseerrggii you're right! good catch! I just pushed a couple of fixes:

  • one for the "All transfers" page (added a test case too)
  • proper settings for config.hosts

Now all green again, even code coverage and coverage diff ✅

@sseerrggii
Copy link
Contributor

@markets @sauloperez I think it's ready to merge

Who should do the merge?

@sauloperez sauloperez merged commit d296ee9 into develop Jan 4, 2021
@sauloperez sauloperez deleted the rails_6 branch January 4, 2021 12:16
@markets
Copy link
Collaborator Author

markets commented Jan 4, 2021

🆒 😎 thanks! Rails 6 is in da house 🎉

@sauloperez sauloperez mentioned this pull request Jan 5, 2021
@sauloperez
Copy link
Collaborator

Already live in production 😎 although I took us v3.1.0 and v3.1.1 to get it right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update dependencies
3 participants