Skip to content

Infrastructure to invoke a backtrace cleaner for failed jobs backtraces #146

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 5 commits into from
Oct 29, 2024

Conversation

hms
Copy link
Contributor

@hms hms commented Aug 6, 2024

Adds a button to allow invoking a backtrace_cleaner on the backtrace of a failed job.

  • Defaults to the standard Rails backtrace_cleaner
  • Can be overridden with a user supplied backtrace_cleaner via an initializer

@hms hms force-pushed the backtrace_cleaner branch from 2abcc09 to d44678f Compare August 14, 2024 19:41
@hms
Copy link
Contributor Author

hms commented Aug 16, 2024

@rosa Could I get this looked at and any feedback necessary to move it forward?

Hal

@rosa
Copy link
Member

rosa commented Aug 17, 2024

@hms yes! Sorry for the delay, I didn't have time last week to look at it but I had it in my plans. I'll try to review next week.

Copy link
Member

@rosa rosa left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your patience, @hms. Very nice! I've left some comments for your consideration 😊

@hms hms force-pushed the backtrace_cleaner branch from d44678f to f6403aa Compare August 22, 2024 19:56
@hms
Copy link
Contributor Author

hms commented Aug 22, 2024

@rosa

Hopefully I didn't cross a complexity line with this update. But I do believe I have addressed your valuable feedback and a little more.

@hms

@hms
Copy link
Contributor Author

hms commented Oct 16, 2024

@rosa Are there still outstanding issues that I haven't addressed? I would love for this to get pushed so I can get back onto the mainline releases.

hms added 5 commits October 16, 2024 16:54
…obs screen

* Defaults to the standard Rails backtrace_cleaner
* Can be overridden with a user supplied backtrace_cleaner via an initializer
I inadvertantly early initialized the Rails.backtrace_cleaner as part
of setting MissionControl wide defaults.  This commit uses a proc
as the default so the database cleaner will be initialized in the
Application environment (vs. the Gems's)

Fixed a typo where I referenced the Gems state instead of the Applicaations
state to access the database cleaner
* (Hopefully) addressed @rosa's feedback on project style, code quality, and implementation requirements
* Updated the design to support both a global MissionControl::Jobs wide backtrace cleaner and
  Application/Server specific backtrace cleaners
* With this commit, the implementation relies less on jobs_helper.rb and
  more on a new before_action in the jobs_controller.
* Added more UI testing for compleness
* Moves defaults handling from MissionControl::Jobs::Application to
  MissionControl::Jobs::Server to minimize impact of this new
  feature on the existing implementation and tests.

  This fixes the unit test failure that I missed in the previous commit.

* Uses @server rather than looking it up via params in the JobsController.
  This allows reverting all changes to the JobsController.
@hms hms force-pushed the backtrace_cleaner branch from f68b461 to 927ae8c Compare October 17, 2024 00:03
@rosa
Copy link
Member

rosa commented Oct 29, 2024

@hms sorry for the delay! All great now 👍 👍 Thanks for your patience!

@rosa rosa merged commit 9923a74 into rails:main Oct 29, 2024
5 checks passed
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.

2 participants