Skip to content
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

Fix execution time checking by keeping lastUpdatedTime in db #1573

Merged
merged 3 commits into from Mar 4, 2024

Conversation

ikreymer
Copy link
Member

@ikreymer ikreymer commented Mar 4, 2024

Instead of relying on status to keep track of lastUpdatedTime, just keep track of it in the db to ensure it is property updated each time. Fixes issues on not counting execution time while avoiding previous issues of double-counting

@ikreymer ikreymer requested a review from tw4l March 4, 2024 19:55
@ikreymer ikreymer changed the base branch from main to v1.9.3-work March 4, 2024 19:59
@Shrinks99 Shrinks99 added this to the v1.9.3 milestone Mar 4, 2024
Copy link
Contributor

@tw4l tw4l left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the fix! Tested locally.

I'm wondering if we could get rid of status.lastUpdatedTime entirely if we're storing it in the db.

The only other place it appears to be used in the operator is is_crawl_stopping, where we'd probably be better served fetching the last updated time from the database for the same reasons as the changes in this PR?

@tw4l
Copy link
Contributor

tw4l commented Mar 4, 2024

We decided to do any updates to status.lastUpdatedTime in future refactors if necessary.

@tw4l tw4l merged commit 060cfcf into v1.9.3-work Mar 4, 2024
2 checks passed
@tw4l tw4l deleted the fix-last-updated-op-check branch March 4, 2024 21:49
ikreymer added a commit that referenced this pull request Mar 6, 2024
Instead of relying on status to keep track of lastUpdatedTime, just keep
track of it in the db to ensure it is property updated each time. Fixes
issues on not counting execution time while avoiding previous issues of
double-counting
ikreymer added a commit that referenced this pull request Mar 6, 2024
Instead of relying on status to keep track of lastUpdatedTime, just keep
track of it in the db to ensure it is property updated each time. Fixes
issues on not counting execution time while avoiding previous issues of
double-counting
ikreymer added a commit that referenced this pull request Mar 6, 2024
- Fix execution time checking by keeping lastUpdatedTime in db by
@ikreymer in #1573
- disable postcss-lit for var css
- Prevent closing tooltips from closing collection share dialog by
@SuaYoo in #1579
- Fix pending exclusion pagination by @SuaYoo in
#1578
- Fix regex escape in exclusion editor text match by @SuaYoo in
#1577

---------
Co-authored-by: emma <hi@emma.cafe>
Co-authored-by: sua yoo <sua@webrecorder.org>
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.

None yet

3 participants