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

Upgrade to rails 6.0.4.4 #249

Merged
merged 18 commits into from
Mar 2, 2022
Merged

Upgrade to rails 6.0.4.4 #249

merged 18 commits into from
Mar 2, 2022

Conversation

waldofouche
Copy link
Contributor

@waldofouche waldofouche commented Feb 23, 2022

Latest rails 6 releases

Tests passed:

2 deprecation warnings total

Finished in 2 minutes 7.7 seconds (files took 11.19 seconds to load)
917 examples, 0 failures

Guard boots without issues:

➜  reporting-service git:(upgrade/rails) bundle exec guard
Notice: Your terminal-notifier is older than what terminal-notifier-guard supports, consider upgrading.
Notice: Your terminal-notifier is older than what terminal-notifier-guard supports, consider upgrading.
12:05:35 - INFO - Bundle already up-to-date
12:05:35 - INFO - Guard::RSpec is running
12:05:35 - INFO - Inspecting Ruby code style of all files
invalid option: -R
For usage information, use --help
12:05:36 - ERROR - The following exception occurred while running guard-rubocop: /Users/waldofouche/.rbenv/versions/2.7.4/lib/ruby/2.7.0/json/common.rb:156:in `parse' 783: unexpected token at '' (JSON::ParserError)
12:05:37 - INFO - 0 brakeman findings
Notice: Your terminal-notifier is older than what terminal-notifier-guard supports, consider upgrading.
Notice: Your terminal-notifier is older than what terminal-notifier-guard supports, consider upgrading.
12:05:37 - ERROR - Notification failed for Notiffany::Notifier: TerminalNotifier not installed. Please do so by running `brew install terminal-notifier`
12:05:37 - INFO - Guard is now watching at '/Users/waldofouche/dev/reporting-service'

Part of https://github.com/ausaccessfed/stockpile/issues/101

Latest rails 6 releases
Avoid using OpenStruct; use Struct, Hash, a class or test doubles
instead.
@waldofouche

This comment was marked as resolved.

@waldofouche

This comment was marked as resolved.

gumboot/strap.rb:28:in `ensure_database': Only supports mysql2 adapter
Implementing the fix causes errors, instead, suppressing the check
@waldofouche

This comment was marked as outdated.

@waldofouche waldofouche marked this pull request as ready for review February 23, 2022 04:32
@waldofouche waldofouche requested a review from a team February 23, 2022 04:32
@@ -2,7 +2,7 @@

require 'mail'
require 'aws-sdk-sqs'

# rubocop:disable Style/OpenStructUse
Copy link
Contributor

@ArthurZheng ArthurZheng Feb 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got rid of OpenStruct in rapid-idp-manager.
Please commit here: https://github.com/ausaccessfed/rapid-idp-manager/pull/1135/commits/68ac2fc6536e5d2a43e948b00232c63e5dd08d6d for details. It's quite a pain to fix it and update all the tests. Just an idea for you. Not sure if it is worth the effort here.

https://github.com/ausaccessfed/rapid-idp-manager/pull/1135/commits/68ac2fc6536e5d2a43e948b00232c63e5dd08d6d

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I am unsure if it is worth the effort here, since this service is planned to die according to: https://github.com/ausaccessfed/stockpile/issues/101

Should I go down this rabbit hole @rianniello ? Happy to if required

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO not worthwhile

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not worth the effort, though good to get this off old rails in meantime 👍)

@waldofouche

This comment was marked as outdated.

@waldofouche waldofouche marked this pull request as draft February 28, 2022 06:25
@matthew-puku
Copy link
Contributor

Might be easier just go to Rails 6.0.x, reporting service should be well and truly dead by the time that hits EOL

@waldofouche
Copy link
Contributor Author

Might be easier just go to Rails 6.0.x, reporting service should be well and truly dead by the time that hits EOL

I agree, will fix up this PR to only go to 6.0.4.4

@waldofouche waldofouche changed the title Upgrade to rails 6.1.4.4 Upgrade to rails 6.0.4.4 Mar 1, 2022
@waldofouche

This comment was marked as resolved.

Used to fix:
DEPRECATION WARNING: Static attributes will be removed in FactoryBot 5.0.
Please use dynamic attributes instead by wrapping the attribute value in
a block:
@waldofouche
Copy link
Contributor Author

Been digging at that for a while, maybe the caching has changed? Would you mind taking a look for me when you have a moment @matthew-puku ? Bit lost

@matthew-puku
Copy link
Contributor

@waldofouche Tests pass now. Rubocop is very unhappy, though. I would probably just remove it now that you've gotten what you wanted from it.

@waldofouche
Copy link
Contributor Author

Thanks for your help @matthew-puku , I agree, I'll remove rubocop now

No longer required
@waldofouche waldofouche marked this pull request as ready for review March 2, 2022 01:46
config/environments/production.rb Outdated Show resolved Hide resolved
bin/setup Show resolved Hide resolved
Copy link
Contributor

@matthew-puku matthew-puku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks alright to me. reporting-service isn't super important so I'm pretty happy to just send this to test and see what it does. Less need to comb over the diff as rigorously as usual 🤷

@waldofouche
Copy link
Contributor Author

Agree @matthew-puku , if the build fails we can just revert and the errors will help us troubleshoot

@waldofouche waldofouche merged commit d9cfe90 into develop Mar 2, 2022
@waldofouche waldofouche deleted the upgrade/rails branch March 2, 2022 02:28
@waldofouche
Copy link
Contributor Author

waldofouche commented Mar 2, 2022

Having no issues using reporting service in test currently, I think everything is all good here @matthew-puku

https://reporting.test.aaf.edu.au/dashboard

@rianniello
Copy link
Contributor

👍

vladimir-mencl-eresearch added a commit to REANNZ/reporting-service that referenced this pull request Mar 7, 2022
* aaf-develop:
  Add platform `x86_64-linux` (ausaccessfed#252)
  Upgrade to rails `6.0.4.4` (ausaccessfed#249)

Resolved-Conflicts:
* config/environments/production.rb
  - adjacent changes: config.log_level changed to :info locally in 5e1e309
vladimir-mencl-eresearch added a commit to REANNZ/reporting-service that referenced this pull request Mar 7, 2022
* code-develop:
  Version: 1.4.5-tuakiri2
  Add platform `x86_64-linux` (ausaccessfed#252)
  Upgrade to rails `6.0.4.4` (ausaccessfed#249)
@vladimir-mencl-eresearch
Copy link
Collaborator

Hi,

I've just rolled it out into production - thanks for the work.

Note that this also upgraded the execjs gem from 2.7.0 to 2.8.1 "again". This version of execjs drops support for therubyracer - this caused problems earlier, reported in #240 - and back then, it was solved by reverting execjs back to 2.7.0.

I have now solved it by just installing nodejs and that works, so not an issue anymore.

But as therubyracer is no longer used, it might be worth removing it from the dependencies - it's still explicitly listed in Gemfile: https://github.com/ausaccessfed/reporting-service/blob/develop/Gemfile#L11

Cheers,
Vlad

waldofouche added a commit that referenced this pull request Mar 8, 2022
@waldofouche waldofouche mentioned this pull request Mar 8, 2022
James-REANNZ added a commit to REANNZ/reporting-service that referenced this pull request Aug 15, 2022
* aaf-develop:
  Bump nokogiri from 1.13.4 to 1.13.6 (ausaccessfed#259)
  Bump rack from 2.2.3 to 2.2.3.1 (ausaccessfed#260)
  fix: mysqld_safe: command not found (ausaccessfed#258)
  Bump nokogiri from 1.13.3 to 1.13.4 (ausaccessfed#256)
  Bump puma from 5.6.2 to 5.6.4 (ausaccessfed#255)
  Gemfile updates (ausaccessfed#254)
  Add platform `x86_64-linux` (ausaccessfed#252)
  Upgrade to rails `6.0.4.4` (ausaccessfed#249)
  Bump nokogiri from 1.12.5 to 1.13.3
  Bump puma from 5.5.2 to 5.6.2
  Update puma to 5.5.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants