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

hyrax-migrator update 6-30-2020 #1228

Merged
merged 7 commits into from Jul 2, 2020
Merged

Conversation

luisgreg99
Copy link

@luisgreg99 luisgreg99 commented Jul 1, 2020

hyrax-migrator update:

@luisgreg99 luisgreg99 changed the title hyrax-migrator update 6-30-2020 [WIP] hyrax-migrator update 6-30-2020 Jul 1, 2020
@luisgreg99
Copy link
Author

luisgreg99 commented Jul 2, 2020

After pulling the latest update for hyrax-migrator, we start getting uninitialized constant Rack::Protection in this line config/initializers/sidekiq.rb#L7 locally and on circle:

NameError: uninitialized constant Rack::Protection
/home/circleci/app/config/initializers/sidekiq.rb:7:in `block in <top (required)>'
/home/circleci/app/config/initializers/sidekiq.rb:6:in `class_eval'
/home/circleci/app/config/initializers/sidekiq.rb:6:in `<top (required)>'

It turns out this hyrax-migrator update also bumps sidekiq to 6.1.0, which recently removed rack-protection from its dependencies according to the release changes. (related PR: 4588).

I think we have a few options to address this issue:

  1. Revert and set sidekiq to use the previous version that works: 6.0.7
  2. Install Rack-protection gem to fix the NameError in this line config/initializers/sidekiq.rb#L7. It looks like rack-protection gem is now part of sinatra
  3. Remove this code block config/initializers/sidekiq.rb#L6-L8 altogether. I believe this code block was a workaround to address a known forbidden rack-protection issue in older versions of sidekiq from this ticket [SOLUTION] Getting 403 Forbidden in Production - Rack-protection & Nginx
    #2560
    . Also Sidekiq Pro panel inaccessible after upgrading to Rails 6.0.3.2 sidekiq/sidekiq#4613 (comment) recommends removing similar calls in the initializers, but I'm not sure if removing it would raise the forbidden issue with our configuration.

I think option (1) setting the version to sidekiq 6.0.7 seems like a simpler or safer solution for now.

@luisgreg99 luisgreg99 requested a review from CGillen July 2, 2020 02:11
@luisgreg99 luisgreg99 changed the title [WIP] hyrax-migrator update 6-30-2020 hyrax-migrator update 6-30-2020 Jul 2, 2020
@CGillen CGillen merged commit ded4053 into master Jul 2, 2020
@CGillen CGillen deleted the update/hyrax-migrator-6-30-2020 branch July 2, 2020 14:13
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.

None yet

2 participants