Skip to content

Conversation

@ron-shinall
Copy link
Contributor

@ron-shinall ron-shinall commented Nov 4, 2024

This PR adds a new destroy_after property, and also introduces the SolidErrors::Cleaner model to handle the work.

Motivation:
It would be nice if Solid Errors could be "self cleaning" by periodically destroying old resolved records.

@fractaledmind let me know your thoughts on this.

  1. I've introduced a new model to handle the work, and I've picked an arbitrary number of 100 as the occurrence id interval to execute the deletions (execute the deletions after every 100 occurrence inserts)
  2. It is currently scoped to only delete resolved errors & occurrences associated with resolved errors
  3. I've used .delete_all to keep it fast
  4. I've used a transaction - probably not be necessary, but it is nice that it keeps it all together
  5. Maybe a different model name should be used? SolidErrors::Maintainer, SolidErrors::Destroyer, etc.?
  6. In the process of designing the tests, I also did the following:
    6.1 Identified several improvements to the test configuration & test dummy app
    6.2 Simplified the database structure in the test dummy app & removed the databases from the repo
    6.3 Renamed the existing test to conform to common Rails conventions (solid_errors_test.rb rather than test_solid_errors.rb)

@fractaledmind
Copy link
Owner

Just finishing up with RubyConf, will give this a review shortly

@ron-shinall
Copy link
Contributor Author

ron-shinall commented Nov 17, 2024

I've been thinking about the scope for destroying records. It's currently scoped only to resolved Errors. I believe it makes more sense to either:

  1. Remove the scope and treat it like a "sharp knife"...if that config value is set, unconditionally destroy all matching records
  2. Convert it to a hash and allow the user to specify the scope, perhaps something like this:
config.solid_errors.destroy_after = { duration: 30.days, only: :resolved }
config.solid_errors.destroy_after = { duration: 30.days, only: :unresolved } # not sure why this would be used
config.solid_errors.destroy_after = { duration: 30.days } # all records older than duration

@fractaledmind
Copy link
Owner

Needed to make some tweaks, so opened #79, but this feature is live in v0.7.0 now. Thanks

@ron-shinall
Copy link
Contributor Author

@fractaledmind thanks so much!

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