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

Update Mastodon to Rails 6.1 #15910

Merged
merged 16 commits into from
Mar 24, 2021
Merged

Conversation

ClearlyClaire
Copy link
Contributor

@ClearlyClaire ClearlyClaire commented Mar 15, 2021

PRs to merge first:

PRs to consider first:

This is work in progress, some tests are still failing:

Other broken things to look fix:

Nice to have:

This PR also does not switch to the new framework defaults yet, this means, among other things, that zeitwerk isn't enabled yet, and will require more work.

@ClearlyClaire ClearlyClaire force-pushed the features/rails-6 branch 3 times, most recently from 160be28 to c05a891 Compare March 15, 2021 23:21
@ClearlyClaire ClearlyClaire force-pushed the features/rails-6 branch 3 times, most recently from 896b26d to 77a0f68 Compare March 17, 2021 12:09
@ClearlyClaire
Copy link
Contributor Author

To elaborate on the “hooks surrounding database setup seem to have changed somehow”, when fiddling with my test setup, I often get the following:

PG::UndefinedFunction: ERROR: function timestamp_id(text) does not exist
LINE 1: ...E TABLE "encrypted_messages" ("id" bigint DEFAULT timestamp_...

This doesn't seem to occur in the CI, but still, something is off and I need to find what before this PR is ready.

@ClearlyClaire ClearlyClaire marked this pull request as ready for review March 19, 2021 13:34
@ClearlyClaire
Copy link
Contributor Author

Marked as ready for review, as I'm fairly happy with how it looks now, with the following caveats:

  • it hasn't been tested in a production environment yet
  • I'm not super happy with having to switch with unreleased forks for a few gems, but they are currently unmaintained

@ClearlyClaire ClearlyClaire changed the title Update Mastodon to Rails 6 Update Mastodon to Rails 6.1 Mar 19, 2021
@ClearlyClaire
Copy link
Contributor Author

I have found no regression so far, but clearing the Rails cache is required as some of the stored objects are Rails models that don't load quite the same on Rails 6.1.

@Gargron Gargron merged commit cbd0ee1 into mastodon:main Mar 24, 2021
@zunda
Copy link
Contributor

zunda commented Mar 25, 2021

As ClearlyClaire wrote, this change seems to make the data in Rails' cache inconsistent with the code resulting with errors like below. Clearing the cache with

tootctl cache clear

seems to have resolved the inconsistency.

method=GET path=/users/zundan/statuses/105946269594904129 format=json controller=StatusesController action=show status=500 error='ActionController::UrlGenerationError: No route matches {:action=>"show", :controller=>"emojis", :id=>#<CustomEmoji id: 183, shortcode: "saba", domain: nil, image_file_name: "food_kandume_saba.png", image_content_type: "image/png", image_file_size: 49717, image_updated_at: "2017-10-07 00:27:30.774446000 +0000", created_at: "2017-09-29 01:28:38.632698000 +0000", updated_at: "2017-10-07 00:27:31.124663000 +0000", disabled: false, uri: nil, image_remote_url: nil, visible_in_picker: true, category_id: nil, image_storage_schema_version: nil>}, possible unmatched constraints: [:id]
Did you mean?  emoji_path
new_admin_custom_emoji_url
new_admin_custom_emoji_path
batch_admin_custom_emojis_url' duration=519.68 view=0.00 db=224.12 key=acct:ClworldF@md.ggtea.org

ActionController::UrlGenerationError (No route matches {:action=>"show", :controller=>"emojis", :id=>#<CustomEmoji id: 183, shortcode: "saba", domain: nil, image_file_name: "food_kandume_saba.png", image_content_type: "image/png", image_file_size: 49717, image_updated_at: "2017-10-07 00:27:30.774446000 +0000", created_at: "2017-09-29 01:28:38.632698000 +0000", updated_at: "2017-10-07 00:27:31.124663000 +0000", disabled: false, uri: nil, image_remote_url: nil, visible_in_picker: true, category_id: nil, image_storage_schema_version: nil>}, possible unmatched constraints: [:id]
Did you mean?  emoji_path
new_admin_custom_emoji_url
new_admin_custom_emoji_path
batch_admin_custom_emojis_url):

app/lib/activitypub/tag_manager.rb:43:in `uri_for'
app/serializers/activitypub/emoji_serializer.rb:13:in `id'
app/lib/activitypub/serializer.rb:36:in `serializable_hash'
app/lib/activitypub/serializer.rb:36:in `serializable_hash'
app/lib/activitypub/adapter.rb:42:in `serializable_hash'
app/controllers/concerns/cache_concern.rb:22:in `render_with_cache'
app/controllers/statuses_controller.rb:38:in `block (2 levels) in show'
app/controllers/statuses_controller.rb:29:in `show'

chrisguida pushed a commit to Start9Labs/mastodon that referenced this pull request Feb 26, 2022
* Update devise-two-factor to unreleased fork for Rails 6 support

Update tests to match new `rotp` version.

* Update nsa gem to unreleased fork for Rails 6 support

* Update rails to 6.1.3 and rails-i18n to 6.0

* Update to unreleased fork of pluck_each for Ruby 6 support

* Run "rails app:update"

* Add missing ActiveStorage config file

* Use config.ssl_options instead of removed ApplicationController#force_ssl

Disabled force_ssl-related tests as they do not seem to be easily testable
anymore.

* Fix nonce directives by removing Rails 5 specific monkey-patching

* Fix fixture_file_upload deprecation warning

* Fix yield-based test failing with Rails 6

* Use Rails 6's index_with when possible

* Use ActiveRecord::Cache::Store#delete_multi from Rails 6

This will yield better performances when deleting an account

* Disable Rails 6.1's automatic preload link headers

Since Rails 6.1, ActionView adds preload links for javascript files
in the Links header per default.

In our case, that will bloat headers too much and potentially cause
issues with reverse proxies. Furhermore, we don't need those links,
as we already output them as HTML link tags.

* Switch to Rails 6.0 default config

* Switch to Rails 6.1 default config

* Do not include autoload paths in the load path
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.

3 participants