Run 500 iterations instead of 100, you should see more stable results
5 Likes
Iāve been running the bench.rb script with 500 iterations. Maybe I should bump it to 1000?
Maybe amend it to do a longer warmup, results should be ultra stable we must fix our script
4 Likes
Just fixed a couple of bugs in our script. It turns out that our admin routes were actually throwing 500s which lead to incorrect benchmark results.
https://github.com/discourse/discourse/commit/26c6447161853a96694ce7c8dda689d33849cc52
I added a guard in the script to ensure that the routes weāre hitting are returning 200 before we start benchmarking.
12 Likes
Rails 5.1 is merged into Discourse.
One notable change which was a source of pain in development for me is that Sidekiq supports live reloading with Rails 5 but dependencies in each file has to be required properly. Otherwise, the job will be stucked in the Sidekiq queue. Iāve fixed most that I can find but if you run into any other job, feel free to let me know.
https://github.com/discourse/discourse/commit/23b787e0a6a695ff8b70d095eb6d0ce603f28f15
14 Likes
Is it good time to introduce microservices to speed up main actions?
Rails 5 broke a few of my plugins because of the change from before_filter to before_action.
I did a quick search through GitHub - discourse/all-the-plugins for before_filter and found these other ones that will be broken:
./plugins/babble/app/controllers/posts_controller.rb:4: before_filter :ensure_logged_in, except: :index
./plugins/babble/app/controllers/topics_controller.rb:4: before_filter :set_default_id, only: :default
./plugins/babble/app/controllers/topics_controller.rb:5: before_filter :ensure_logged_in, except: [:show, :index]
./plugins/blog/app/controllers/blog/application_controller.rb:8: before_filter :cache_anon
./plugins/discourse-admin-statistics-digest/lib/admin_statistics_digest/active_user.rb:35: signed_up_before_filter = if filters.signed_up_before
./plugins/discourse-admin-statistics-digest/lib/admin_statistics_digest/active_user.rb:88: #{ signed_up_before_filter }
./plugins/discourse-atom-receiver/app/controllers/discourse_atom_receiver/atom_file_controller.rb:5: skip_before_filter :check_xhr
./plugins/discourse-blogger-plugin/plugin.rb:33: skip_before_filter :check_xhr, :preload_json, :verify_authenticity_token
./plugins/discourse-blogger-plugin/plugin.rb:35:# before_filter :ensure_embeddable, except: [ :info ]
./plugins/discourse-donations/app/controllers/discourse_donations/charges_controller.rb:7: skip_before_filter :verify_authenticity_token, only: [:create]
./plugins/discourse-layouts/controllers/widget.rb:4: before_filter :ensure_admin
./plugins/discourse-plugin-permalink/integrate.rb:35: klass.skip_before_filter :check_xhr, only: [:permalink]
./plugins/discourse-plugin-shared-edit/plugin.rb:59: before_filter :can_change_share_edit?
./plugins/discourse-qa/plugin.rb:39: before_filter :check_if_voted, only: :create
./plugins/discourse-reset-bump/plugin.rb:42: # You'll find other plugins and Discourse code using before_filter instead of
./plugins/discourse-reset-bump/plugin.rb:44: # makes 'before_filter' deprecated and start causing warnings, and Rails 5.1 will remove
./plugins/discourse-slackdoor/plugin.rb:24: before_filter :slackdoor_enabled?
./plugins/discourse-slackdoor/plugin.rb:25: before_filter :slackdoor_username_present?
./plugins/discourse-slackdoor/plugin.rb:26: before_filter :slackdoor_token_valid?
./plugins/discourse-user-feedback/app/controllers/ratings_controller.rb:4: before_filter :ensure_topic_existence
./plugins/dl-license-keys/app/controllers/dl_license_keys/license_users_controller.rb:6: skip_before_filter :check_xhr, only: [:validate]
./plugins/retort/plugin.rb:30: before_filter :verify_post_and_user, only: :update
Some of these might be obsolete - if so maybe they can be removed from the all-the-plugins repo, and moved to #plugin:broken-plugin to avoid any future confusion.
babble @gdpelican
blog @sam
(official?) discourse-atom-receiver
discourse-blogger-plugin @kcoop
discourse-donations @rimian
discourse-layouts @angus
discourse-plugin-permalink @lightyear
discourse-qa @angus
discourse-reset-bump (just a comment)@LeoDavidson
discourse-slackdoor @mcwumbly
discourse-user-feedback @jafeth.diazc
dl-license-keys @joebuhlig
retort @gdpelican
12 Likes
@jafeth.diazc @joebuhlig @gdpelican see above post (could only mention 10 people at a time)
1 Like
Not obsolete yet, I donāt think. This functionality doesnāt exist yet in the new slack plugin, does it?
Will put it on my list to fixā¦ Hopefully soonā¦
1 Like
Nope, not included in slack-official or chat-integration
Thanks for calling this out David. I hadnāt noticed Rails 5 was official yet so hadnāt touched this at all. Now it looks I need to hustle. 
4 Likes
Hopefully this is all thatās needed:
https://github.com/mcwumbly/discourse-slackdoor/commit/26ec8b4bf84aca08596d85c5f2fc8c8f1efa5d12
(Wish I had CI set up for this thing).
5 Likes
Nescis quid serus vesper vehat 
Thanks David.
Note that, if you have written specs, all
xhr :method, :action, params
has to be re-written as
method :action, params: params, format: :json
6 Likes
Thanks for the heads up @david, Iāve 
my things. 
Is it safe to do an update of my forum at the moment? Are people finding any instability / upgrade issues post-Rails-upgrade?
Hang fire for now. @tgxworld is investigating a possible instability.
3 Likes
Looks like @tgxworld has concluded his investigation. Should be safe to update, but it couldnāt hurt to wait a day or two more while we finish ironing out any additional unexpected issues.
5 Likes
And if you use any non official plugins youāll want to be especially careful.
5 Likes
Iām with @pfaffman here. I just finished pushing updates to the majority of my plugins to cover this but Iām sure there are others still working on it.
6 Likes
Thank you for your efforts! Does this mean that the username search related PR can finally be merged in? 
1 Like