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.

Sign up for GitHub

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

fix: Allow customizing the rake task regex to avoid starting the reporter #220

Merged
merged 7 commits into from
Oct 16, 2024
Merged

fix: Allow customizing the rake task regex to avoid starting the reporter #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8f36030
fix: Allow customizing the rake task regex to avoid starting the repo…
adamlogic Oct 10, 2024
92a34b9
standard formatting
adamlogic Oct 10, 2024
ba932c2
Merge branch 'main' into config-rake-task
adamlogic Oct 15, 2024
c649d9c
Fix frozen string for Ruby 2.x
adamlogic Oct 15, 2024
e54cf5a
Fix frozen string for Ruby 2.x
adamlogic Oct 15, 2024
f4f9fd9
rake_task_ignore_regex is a Rails-specific option
adamlogic Oct 15, 2024
83ef97a
standard
adamlogic Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion judoscale-rails/lib/judoscale/rails/config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
module Judoscale
module Rails
module Config
attr_accessor :start_reporter_after_initialize
attr_accessor :start_reporter_after_initialize, :rake_task_ignore_regex

def reset
super
@start_reporter_after_initialize = true
@rake_task_ignore_regex = /assets:|db:/
end
end

Expand Down
2 changes: 1 addition & 1 deletion judoscale-rails/lib/judoscale/rails/railtie.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def judoscale_config
config.after_initialize do
if in_rails_console_or_runner?
logger.debug "No reporting since we're in a Rails console or runner process"
elsif in_rake_task?(/assets:|db:/)
elsif in_rake_task?(judoscale_config.rake_task_ignore_regex)
logger.debug "No reporting since we're in a build process"
elsif judoscale_config.start_reporter_after_initialize
Reporter.start
Expand Down
4 changes: 4 additions & 0 deletions judoscale-rails/test/config_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,9 @@ module Judoscale
it "adds the start_reporter_after_initialize config option" do
_(::Judoscale::Config.instance.start_reporter_after_initialize).must_equal true
end

it "adds the rake_task_ignore_regex config option" do
_(::Judoscale::Config.instance.rake_task_ignore_regex).must_equal(/assets:|db:/)
end
end
end
1 change: 1 addition & 0 deletions sample-apps/rails-sample/config/initializers/judoscale.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
Judoscale.configure do |config|
# Open https://judoscale-adapter-mock.requestcatcher.com in a browser to monitor requests
config.api_base_url = ENV["JUDOSCALE_URL"] || "https://judoscale-adapter-mock.requestcatcher.com"
config.rake_task_ignore_regex = /assets:|db:|middleware/
Copy link
Member

Choose a reason for hiding this comment

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

Does it seem that "middleware" is default enough to warrant we skipping that by default as well? Or do you think we could run into some unexpected issue if we were to add it now?

Copy link
Collaborator Author

@adamlogic adamlogic Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think we'd run into any issues by including middleware by default, but for now I think I'd rather leave it out. It's not typically run in build processes or prod environments, and it's a quick task as opposed to a task that could take a while.

I don't think our default rake_task_ignore_regex needs to be exhaustive. The goal is really to avoid starting the reporter during common build processes.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha, sounds good.

# config.start_reporter_after_initialize = false
end