re-add missing condition for --skip-segments-today #16777
Conversation
diosmosis
added
not-in-changelog
|
@diosmosis two Integration tests are failing. Guess it's unrelated to this PR. But would be good to fix them.... |
|
@diosmosis what happens if there is an Would the segment still be archived the next day after midnight? Or maybe because there's no archive_invalidation entry it wold not be archived in that case? Could you double check this maybe? |
|
@tsteur I see, will add the check to QueueConsumer as well and test both cases. |
|
@diosmosis let me know when it's pushed and we'll include it in 4.0.2. |
|
@tsteur finished the test |
core/Archive.php
Outdated
| @@ -234,6 +234,15 @@ public static function factory(Segment $segment, array $periods, array $idSites, | |||
| $isMultipleDate); | |||
| } | |||
|
|
|||
| // TODO: check period has today, not equals | |||
There was a problem hiding this comment.
@diosmosis is this a todo that still needs work? Looking at code it works fine as we only look at today anyway.
There was a problem hiding this comment.
It's not needed, I must have been thinking of non-day periods, but for some reason we don't skip those archives... oh that's because before if we skipped day, the logic to launch higher periods wouldn't be executed. This will be a problem now correct? If so it's needed.
There was a problem hiding this comment.
Before we were still launching higher periods that include today @diosmosis
|
Tested it quite a few times and worked for me 👍 left one comment regarding a comment in the code. otherwise feel free to merge @diosmosis 🚀 |
Description:
Fixes #16775
The missing condition was the only difference in the code I could see.
Review