[14.0][PERF] dynamic routing #722
Conversation
jbaudoux
force-pushed
the
14-perf-stock_dynamic_routing
branch
from
462cbbc to
78f5c93
Compare
simahawk
left a comment
simahawk
left a comment
There was a problem hiding this comment.
Changes looks good.
I think it's time to upgrade these modules to dev status = production/stable as they are used in prod since years w/o major issues. WDYT?
jbaudoux
force-pushed
the
14-perf-stock_dynamic_routing
branch
from
78f5c93 to
62a4b43
Compare
As the call to _check_entire_pack can be very slow, bypass it's call when pre-computing the dynamic routing pull rules as this is not needed for the computation and will be rollbacked. Also in case of source relocation, ensure to call _check_entire_pack on the recordset of moves and not on each move one by one. This is because _check_entire_pack triggers the check on the picking of the move and so you perform the same check for each move of the picking. When you reserve from source packages you have many moves inside a picking, the check is quite long and this change makes a important performance improvement. When the dynamic routing does not change the source location, do not write the same value on the move as this triggers many useless additional recomputations.
Add test for the rerouting of waiting moves in a pull flow
jbaudoux
force-pushed
the
14-perf-stock_dynamic_routing
branch
from
62a4b43 to
1ea674e
Compare
|
|
There was a feature for the re-routing of waiting moves in the chain (even when the pick location destination is not changed) that was not covered by a test. To be sure the performance changes are not breaking any feature, I have added a test for this. This feature is deprecated in favor of the |
You are no the maintainer 😉 Can you take care of this on another PR? 🙏 |
| if unconfirmed_moves: | ||
| unconfirmed_moves._apply_source_relocate() |
There was a problem hiding this comment.
Does it change something to check the recordset here?
Should we instead do this check in the _apply_source_relocate method, if self is empty return.
There was a problem hiding this comment.
No the method should not be called if not necessary.
Otherwise, all methods in odoo would start with if not self: return which is ugly
There was a problem hiding this comment.
Well, here except the debug log the method will do nothing. And as the Odoo API tends to be "functional-programming" in some way it's OK to call methods on an object without thinking if it's empty or not.
| return new_move | ||
|
|
||
| def _after_apply_source_relocate_rule(self): | ||
| # Hook for stock_dynamic_routing |
There was a problem hiding this comment.
Not sure we need to mention this module in stock_move_source_relocate? It's the job of the glue module (or anything else) to use this hook.
|
/ocabot merge minor |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 481205e. Thanks a lot for contributing to OCA. ❤️ |
5 participants
As the call to _check_entire_pack can be very slow, bypass it's call when pre-computing the dynamic routing pull rules as this is not needed for the computation and will be rollbacked.
Also in case of source relocation, ensure to call _check_entire_pack on the recordset of moves and not on each move one by one. This is because _check_entire_pack triggers the check on the picking of the move and so you perform the same check for each move of the picking. When you reserve from source packages you have many moves inside a picking, the check is quite long and this change makes a important performance improvement.
When the dynamic routing does not change the source location, do not write the same value on the move as this triggers many useless additional recomputations.
cc @mt-software-de