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
add visitfrequency archiver to archive dependent archives #17168
Conversation
sounds good for now 👍 . I don't think it affects any other |
@tsteur will also add an update to automatically invalidate, unless you think that's a bad idea. |
@diosmosis I'm assuming this would mean we rearchive all the data from the last few months not just this report? cc @mattab |
I think we can just re-archive visitfrequency from start of matomo 4 upgrade to now. We just need to trigger the VisitFrequency archiver which will process dependent archives. So we shouldn't need to do every archive. |
👍 sounds good |
added the update file |
$invalidator = StaticContainer::get(ArchiveInvalidator::class); | ||
$invalidator->scheduleReArchiving('all', 'VisitFrequency', null, $dateOfMatomo4Release); |
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.
Will that work when only browser archiving is used?
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.
it shouldn't be necessary for browser archiving, since VisitFrequency.get will be requested and will trigger archiving in that case. this bug should only affect core:archive setups
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.
Makes sense. Looks good to merge then
Description:
Fixes #17123
@tsteur found the fix. The cause is since we call
CoreAdminHome.archiveReports
now instead ofAPI.get
, the dependent archive isn't implicitly created. Adding an archiver for VisitFrequency fixes this, though I'm not sure if we want to maintain the old behavior.Review