-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
stats recompute fixes: #2022
stats recompute fixes: #2022
Conversation
ikreymer
commented
Aug 14, 2024
- fix stats_recompute_last() and stats_recompute_all() to not update the lastCrawl* properties of a crawl workflow if a crawl is running, as those stats now point to the running crawl
- refactor _add_running_curr_crawl_stats() to make it clear stats only updated if crawl is running
- stats_recompute_all() change order to ascending to actually get last crawl, not first!
- fix stats_recompute_last() and stats_recompute_all() to not update the lastCrawl* properties of a crawl workflow if a crawl is running, as those stats now point to the running crawl - refactor _add_running_curr_crawl_stats() to make it clear stats only updated if crawl is running - stats_recompute_all() change order to ascending to actually get last crawl, not first!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one comment
backend/btrixcloud/crawlconfigs.py
Outdated
@@ -596,6 +593,8 @@ async def stats_recompute_last(self, cid: UUID, size: int, inc_crawls: int = 1): | |||
update_query["lastCrawlSize"] = sum( | |||
file_.get("size", 0) for file_ in last_crawl.get("files", []) | |||
) | |||
update_query["lastCrawlStopping"] = last_crawl.get("stopping", False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this always be False like isCrawlRunning
? Otherwise if the crawl were still stopping it would still be in a running state I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, i guess this would reflect if last crawl was stopped manually, though that may be confusing since its referring to last crawl..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, updated as suggested - otherwise too confusing - if we do need that for some reason, could add a lastCrawlStopped
field..
backend/btrixcloud/crawlconfigs.py
Outdated
@@ -972,6 +960,8 @@ async def stats_recompute_all(crawl_configs, crawls, cid: UUID): | |||
update_query["lastStartedByName"] = last_crawl.get("userName") | |||
update_query["lastCrawlState"] = last_crawl.get("state") | |||
update_query["lastCrawlSize"] = last_crawl_size | |||
update_query["lastCrawlStopping"] = last_crawl.get("stopping", False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, I think this should always be False