fix: Fix crawler_runtime not being updated during run and only in the end
#1540
Conversation
github-actions
bot
added
t-tooling
Pijukatel
added
adhoc
| async def __aenter__(self) -> Self: | ||
| self._active = True | ||
| await self._state.initialize() | ||
| self._after_initialize() |
There was a problem hiding this comment.
I am not sure what this was for these dummy statistics. Could you please double-check @janbuchar ?
There was a problem hiding this comment.
It was probably so that record_* methods wouldn't randomly fail during context pipeline execution because of incorrectly initialized state. I assume it's not necessary anymore?
There was a problem hiding this comment.
I haven't seen any failure in tests, and those methods execute. But the same can be seen on master when deleting this line, so I guess it was made redundant by some other change?
Pijukatel
requested review from
janbuchar and
vdusek
and removed request for
vdusek
janbuchar
left a comment
janbuchar
left a comment
There was a problem hiding this comment.
Seems legit, just one comment readability issue
| async def __aenter__(self) -> Self: | ||
| self._active = True | ||
| await self._state.initialize() | ||
| self._after_initialize() |
There was a problem hiding this comment.
It was probably so that record_* methods wouldn't randomly fail during context pipeline execution because of incorrectly initialized state. I assume it's not necessary anymore?
| # Flag to indicate the context state. | ||
| self._active = False | ||
|
|
||
| # Pre-existing runtime offset when importing existing statistics. |
There was a problem hiding this comment.
What does importing existing statistics mean here? Like restoring serialized state from KVS?
There was a problem hiding this comment.
Yes, updated comment.
Description
BasicCrawler.statistics.state.crawler_runtimeto be properly updated on eachBasicCrawler.statistics.calculate()call when the statistics are still active.BasicCrawler.statistics.state.crawler_runtimeonBasicCrawler.statistics.calculate()when the statistics are already deactivated - to avoid confusing differences between logged and persisted state.Issues
Closes: #1541
Testing
Checklist