Introduce support for task staleness threshold#1406
Open
joshmfrankel wants to merge 3 commits intoShopify:mainfrom
Open
Introduce support for task staleness threshold#1406joshmfrankel wants to merge 3 commits intoShopify:mainfrom
joshmfrankel wants to merge 3 commits intoShopify:mainfrom
Conversation
217b6cd to
039de31
Compare
joshmfrankel
commented
Mar 5, 2026
Comment on lines
+79
to
+81
| within page.find("a", text: "Maintenance::OutdatedTask").find(:xpath, "..").sibling(".has-text-warning") do |element| | ||
| assert_text "This task last ran 1 day ago. Consider removing it as it may be outdated." | ||
| end |
Contributor
Author
There was a problem hiding this comment.
Future: Might be worthy of a follow-up PR to separate the various sections on the Task#index page within individual parent divs. That would make the lookup easier for testing as well as organizing the DOM.
jenshenny
reviewed
Mar 10, 2026
Contributor
jenshenny
left a comment
There was a problem hiding this comment.
I think having a way to surface stale tasks would be valuable to have!
* Changed all language of `outdated` to `stale` * Provided an opt-out mechanism by setting MaintenanceTasks.task_staleness_threshold to false * Protected delegated calls to related run being NilClass * Scoped staleness checks to only succeeded Runs
Contributor
Author
|
@jenshenny Great feedback! I believe I've incorporated all suggestions 🎉 |
jenshenny
approved these changes
Mar 19, 2026
Contributor
jenshenny
left a comment
There was a problem hiding this comment.
This looks good to me pending fixing failing CI
| test "#stale? returns `false` when the run is not succeeded" do | ||
| MaintenanceTasks.with(task_staleness_threshold: 1.day) do | ||
| run = Run.create!(task_name: "Maintenance::UpdatePostsTask", ended_at: 2.days.ago, status: :running) | ||
| refute run.stale? |
Contributor
There was a problem hiding this comment.
nit: can we use assert_predicate on these assertions?
Comment on lines
68
to
71
| import_posts_task_succeeded_old: | ||
| task_name: Maintenance::ImportPostsTask | ||
| tick_count: 10 | ||
| tick_total: 10 |
Contributor
There was a problem hiding this comment.
CI is failing since these fields were erroneously deleted
created_at: '01 Jan 2020 00:20:00'
started_at: '01 Jan 2020 00:40:36'
ended_at: '01 Jan 2020 00:52:19'
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Maintenance tasks are ephermeral by nature. This pull request introduces a mechanism to display a gentle message to encourage removal of Tasks which have not run within the configured threshold.
"Maintenance tasks aren't meant to happen on a regular basis. They're used as needed, or as one-offs. Normally maintenance tasks are ephemeral, so they are used briefly and then deleted."
What
task_staleness_thresholdconfigurationOpen Questions