dinkypumpkin
commented
7 years ago Thanks for the PR. I didn't think Perl would roll over to week 0 in that manner, but obviously I was wrong. I'm not sure I'm going to merge this, however. I deprecated --refresh-limit, then un-deprecated it in dev branch, but now it's going back on the scrapheap. If I can't remove it in the next day or so, I'll merge your changes as a stopgap. Thanks again.
EDIT: On second thought, I'll stick with your fix.
When a refresh limit is set and not divisible by 7 days, refreshing programme schedules at the end of the year can incorrectly try to download schedules for all weeks in at least the next year. I detected this problem today because my refresh limit is set to 30 days and get_iplayer attempted to download schedules for all weeks of 2017. This seems to be because today is in "week 52" but starting from 30 days ago and incrementing 7 days at a time means that the Sunday that is in "this week" falls into what Perl calls "week 0" of 2017.
The earliest date I've found in my non-scientific search where this behaviour becomes a significant problem is December 31st 2018, which is a Monday and "week 53". In this case, for users with
$limit_days + 1divisible by 7 get_iplayer will attempt to download schedules for all weeks to the end of 2024. More worrying, for users with neither$limit_daysnor$limit_days + 1divisible by 7 (73% of the possible non-zero refresh limits), get_iplayer will go into what looks like an infinite loop generating the list of schedule weeks it wants to download. I'm sure it could be proven this will be infinite with maths, testing with limits of 1-5 days reached a year of 20000+ without matching the loop escape criteria.I've tested this patch today using
get_iplayer --refresh --refresh-limit 30(confirming thatWARNING: Failed to download programme schedule ...messages for weeks in 2017 no longer appear) on the following platforms: