miscellaneous cleanup of ilsgateway migration code #6421
Conversation
| # create stockout alerts | ||
| product_data = ProductAvailabilityData.objects.filter(supply_point=facility_id, date=date, without_stock=1) | ||
| for p in product_data: | ||
| create_multilevel_alert(facility_id, date, const.PRODUCT_STOCKOUT, {'org': facility_id, 'product': p.product}) |
There was a problem hiding this comment.
E501 line too long (118 > 115 characters)
| from custom.ilsgateway.tasks import report_run | ||
| from custom.ilsgateway.tanzania.warehouse import updater | ||
|
|
||
| class Command(BaseCommand): |
There was a problem hiding this comment.
E302 expected 2 blank lines, found 1
|
@kkrampa @kaapstorm I think this is good to merge and would be nice to get it in to avoid future conflicts. @kkrampa let me know if you disagree |
|
@czue I tested locally runner and it works as before your changes. Regarding of get_nested_children function luckily problem that you described didn't cause any problems. Maybe we need to change this function later. |
|
👍 |
|
rebased with master - i think this is good to go (feel free to wait for tests) |
| res = chain( | ||
| process_non_facility_warehouse_data.si(org, start_date, end_date, runner, strict) | ||
| for org in chunk | ||
| )() |
There was a problem hiding this comment.
Just curious: Is there a reason we chain these (sequentially) instead of grouping them (in parallel)? i.e.
res = group(
process_non_facility_warehouse_data.si(org, start_date, end_date, runner, strict)
for org in chunk
)()
And could we chain the groups, or stick them all in a single group/chain?, i.e.
res = chain(
group(
process_non_facility_warehouse_data.si(org, start_date, end_date, runner, strict)
for org in chunk
)()
for chunk in non_facilities_chunked_list
)()
res.get()
(Not that it would make a significant performance improvement if we could, because it's all happening in the background queue. Just trying to figure out why we do it the way we do.)
There was a problem hiding this comment.
I'm not sure - it might be because of the memory issues pointed out by @kkrampa here: 598e008#commitcomment-10855444 or it might just never have been considered to run in parallel (this code was just moved around so @kkrampa is best positioned to answer)
|
@kaapstorm Please look at my answer in url that Cory posted. |
|
Ah. Thanks @kkrampa. |
miscellaneous cleanup of ilsgateway migration code
|
any reason travis didn't run on this? |
|
I think because I rebased and the last commit had |
can be read commit-by-commit
one bugfix in 598e008
the rest of this is mostly just moving stuff around to try and understand it better.
@kkrampa do you want to take a look at this? cc @kaapstorm