@diosmosis opened this Pull Request on November 24th 2020 Member

Description:

Fixes #16775

The missing condition was the only difference in the code I could see.

Review

  • [ ] Functional review done
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@sgiehl commented on November 24th 2020 Member

@diosmosis two Integration tests are failing. Guess it's unrelated to this PR. But would be good to fix them....

@tsteur commented on November 24th 2020 Member

@diosmosis what happens if there is an archive_invalidation entry for this segment that is for today. Then it sends the request to archive. It thinks the segment was archived (but in reality it was skipped), and then it removes the archive_invalidation entry.

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?

@diosmosis commented on November 24th 2020 Member

@tsteur I see, will add the check to QueueConsumer as well and test both cases.

@tsteur commented on November 26th 2020 Member

@diosmosis let me know when it's pushed and we'll include it in 4.0.2.

@diosmosis commented on November 27th 2020 Member

@tsteur finished the test

@tsteur commented on November 30th 2020 Member

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 🚀

This Pull Request was closed on November 30th 2020
Powered by GitHub Issue Mirror