Skip to content

feat: exclude insights from rake tasks config #697

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

Merged
merged 2 commits into from
Jun 5, 2025

Conversation

roelbondoc
Copy link
Contributor

This introduces a new configuration option insights.exclude_rake_tasks. By default this includes the assets:precompile rake task commonly executed in Rails. This can be overridden to include other tasks when needed.

Closes: #694

Before submitting a pull request, please make sure the following is done:

  1. If you've fixed a bug or added code that should be tested, add tests!
  2. Run rake spec in the repository root.
  3. Use a pull request title that conforms to conventional commits.

This introduces a new configuration option
`insights.exclude_rake_tasks`. By default this includes the
`assets:precompile` rake task commonly executed in Rails. This can be
overridden to include other tasks when needed.
@roelbondoc roelbondoc requested a review from stympy June 5, 2025 13:59
@roelbondoc
Copy link
Contributor Author

@stympy Let me know what you think about this instead. This would help with non-rails environments as well that could potentially run into the same issue.

@stympy
Copy link
Member

stympy commented Jun 5, 2025

I like this for replacing the rake check in #695, but I also like having the ActiveRecord check in #695. :) That is, I think it makes sense for the SolidQueue plugin to still check the database connection before it's activated, even with the rake test moved elsewhere.

Have you confirmed that this is checked late enough in the Rails initialization process that Rake.application.top_level_tasks contains 'assets:precompile'?

@roelbondoc
Copy link
Contributor Author

I like this for replacing the rake check in #695, but I also like having the ActiveRecord check in #695. :) That is, I think it makes sense for the SolidQueue plugin to still check the database connection before it's activated, even with the rake test moved elsewhere.

Agreed, that makes sense! I'll update your PR so we keep the ActiveRecord connection check.

Have you confirmed that this is checked late enough in the Rails initialization process that Rake.application.top_level_tasks contains 'assets:precompile'?

Yup, I've tested it locally and it does contain the task.

@roelbondoc roelbondoc merged commit 118034c into master Jun 5, 2025
86 checks passed
@roelbondoc roelbondoc deleted the feat-exclude-from-rake-tasks branch June 5, 2025 15:30
stympy added a commit that referenced this pull request Jul 1, 2025
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.

Insights attempts database connections during asset precompilation
2 participants