-
Notifications
You must be signed in to change notification settings - Fork 11
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
Prefer announced suburbs that have not been updated in 30 days #227
Prefer announced suburbs that have not been updated in 30 days #227
Conversation
Looks good did you just want to remove the commented-out prints? |
Done. Some day, this deserves a unit-test. |
Cheers |
Will need to keep an eye on it to see if 30 days works well. |
I might PR a bit of logging that emits when it tries each of the preferences, so we can tell where it's up to... |
Right now there are only 7 announced suburbs with processing dates older than 30 days:
|
GHA hasn't got those 7 yet - difference in timezone (and the processed_date field doesn't have TZ info). If the GHA keeps running, we should see a few 30-day suburbs popping up every day (about 90/day on average). The rest of the day will be processing the non-announced suburbs. |
Turns out (duh) the number of announced-suburbs that will be processed is more lumpy than that. Edit: See comment/date below Given that we process ~400-500/day now, our full-cycle time will be about 30 days anyway, so I don't think this PR is actually needed (hahah). |
def check_processing_rate():
announced_tally = Counter()
other_tally = Counter()
for state, suburb_list in suburbs.read_all_suburbs().items():
for suburb in suburb_list:
tally = announced_tally if suburb.announced else other_tally
tally[suburb.processed_date.date()] += 1
data = (
(day, announced_tally.get(day), other_tally.get(day))
for day in sorted(announced_tally.keys() | other_tally.keys())
)
print(tabulate(data, headers=["date", "announced", "other"], tablefmt="github")) emits
|
|
Well if it's that we can reliably recheck everything in 30 days the time period for announced could possibly be reduced to something lower or maybe checking something with the announced date. |
I'm not sure I believe anything from NBNco about their announced date :) I'll commit the code above into a PR in adhoc-tools, so we can run it again in a few weeks, see how it's going. |
Per #226
No preference given to closer announced dates - I think processing will be fast enough already.