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

Allow invalidation of All Visits only #22989

Open
wants to merge 8 commits into
base: 5.x-dev
Choose a base branch
from
Open

Allow invalidation of All Visits only #22989

wants to merge 8 commits into from

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jan 27, 2025

Description:

This PR improves and fixes some parts of the core:invalidate-report-data command

  • Allow invalidating All visits segment only by providing --segment=""
    This new option will only invalidate the all visits segment

  • Correctly invalidate all known segments when no segment is provided
    Before only segments with existing archives were invalidated, causing other segments not to be re archived at all.

  • Fixed range invalidation of plugin archives
    While "normal" periods were invalidated correctly when a plugin was provided, this was not correctly done for range archives. Instead the all visits archive was invalidated, causing more re archiving than requested.

  • Improved handling of provided segments
    When segments were provided that did not exist for one of the provided sites, the command previously failed. Now it will invalidate the segment for all sites where it is available, while skipping it for other sites.

The code overall was a restructured and improved. A plenty of new tests for the command should ensure that the correct invalidations are created when command is executed with certain parameters.

Currently "excluded" from the testing for the command is the handling of existing archives. Those should be marked as invalidated and in certain cases also plugin and segment archives should get invalid, depending on the invalidation request.
But I didn't want to extend the scope of this change here too much, so we should consider writing some proper tests for Model::updateArchiveAsInvalidated method another time.

fixes #19947

Review

@sgiehl sgiehl force-pushed the dev-13817 branch 4 times, most recently from 84528ee to bc48aa2 Compare January 31, 2025 14:40
@sgiehl sgiehl marked this pull request as ready for review January 31, 2025 15:10
@sgiehl sgiehl added this to the 5.3.0 milestone Jan 31, 2025
@sgiehl sgiehl requested a review from a team January 31, 2025 15:41
@sgiehl sgiehl added the Needs Review PRs that need a code review label Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalidations of reports command should allow to invalidate "All visits" only
2 participants