Skip to content

Update Job Status Retention Policy#179

Open
ascibisz wants to merge 2 commits intofeature/pass-json-recipefrom
maint/job-status-retention
Open

Update Job Status Retention Policy#179
ascibisz wants to merge 2 commits intofeature/pass-json-recipefrom
maint/job-status-retention

Conversation

@ascibisz
Copy link
Contributor

@ascibisz ascibisz commented Feb 5, 2026

Problem

Since we're using the job_status collection for matching subsequent runs of a recipe to it's results (instead of just providing results for a single run), we should increase the retention policy before we clean up runs.

Link to story or ticket

Solution

  • Change the retention policy for job_status from 24 hours to 30 days
  • Add a step to update the timestamp on an entry in job_status whenever it's read, so we only clear out entries that haven't been accessed at all in 30 days
  • Fix bug where "runs" recipes that have previously been run get stuck in the "STARTING" status, even though the result shows up immediately. This was a simple fix, just adding a call to setJobStatus after our initial call to getJobStatus, so it'll be set with the proper value even if we don't enter the while loop
  • Fix a bug where our error messages can't be converted into JSON because they come through in text form

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 35.06% 676 / 1928
🔵 Statements 35.06% 676 / 1928
🔵 Functions 40.38% 42 / 104
🔵 Branches 73.46% 144 / 196
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
src/App.tsx 0% 0% 0% 0% 1-5, 14-17, 19, 21-22, 24-25, 27-33, 35, 37-39, 41-47, 49-63, 65-81, 83-91, 93-112, 116, 118-136, 138-142, 144-148, 150-169, 171, 173
src/constants/firebase.ts 100% 100% 100% 100%
src/utils/firebase.ts 59.61% 73.91% 62.5% 59.61% 40-43, 82-92, 102-110, 114-127, 130-134, 137-142, 153-154, 239-249, 251-260, 262, 264-268, 270-275
Generated in workflow #228

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://AllenCell.github.io/cellpack-client/pr-preview/pr-179/

Built to branch gh-pages at 2026-02-05 18:22 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@ascibisz ascibisz changed the title set retention for job status to 30 days, update job status timestamp … Update Job Status Retention Policy Feb 5, 2026
@ascibisz ascibisz linked an issue Feb 5, 2026 that may be closed by this pull request
@ascibisz ascibisz requested a review from Copilot February 5, 2026 18:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the retention policy for job status records from 24 hours to 30 days and implements timestamp updates to ensure only truly inactive jobs are cleaned up. It also fixes two bugs related to job status handling and error message serialization.

Changes:

  • Retention policy for job_status increased from 24 hours to 30 days
  • Job status timestamps now update on read to track active usage
  • Fixed status display issue for previously-run recipes
  • Fixed JSON serialization error for failed job responses

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/constants/firebase.ts Updates retention period constant from 24 hours to 30 days
src/utils/firebase.ts Adds new function to update job status timestamps and exports it
src/App.tsx Implements timestamp updates, fixes status setting for cached jobs, and corrects error response handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ascibisz ascibisz requested a review from rugeli February 5, 2026 18:22
@ascibisz ascibisz marked this pull request as ready for review February 5, 2026 18:22
Copy link
Contributor

@rugeli rugeli left a comment

Choose a reason for hiding this comment

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

lgtm! this fits well with our current workflow! this is also making me think (out of scope for this pr) whether we should use "created_at" and last_used/last_accessed fields instead of timestamp. happy to chat about it later

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.

Reassess Retention Policy for job_status collection

2 participants