[10.0] sale_exception: Remove side effect from api.constrains #916
Conversation
guewen
changed the title
|
Travis failed because postgres was not running? 😵 |
|
Hehe, no, but you have to update to latest MQT template: https:/OCA/maintainer-quality-tools/blob/master/sample_files/.travis.yml The reason behind is that Travis has switched its base OS and now it doesn't bundle PostgreSQL by default. |
Thanks! |
|
Hi @guewen , just checking if travis is 💚 to merge but it is 🔴 😢
Could take a look please? Thanks |
|
Hi @rafaelbn, The travis error is unrelated, a test failing in sale_triple_discount :( |
|
you should do a rebase |
The method called by 'sale_check_exception' has a side effect, it writes on 'exception.rule' + on the Many2many relation between it and sale.order(.line). When decorated by @api.constrains, any error during the method will be caught and re-raised as "ValidationError". This part of code is very prone to concurrent updates as 2 sales having the same exception will both write on the same 'exception.rule'. A concurrent update (OperationalError) is re-raised as ValidationError, and then is not retried properly. Calling the same method in create/write has the same effect than @api.constrains without shadowing the exception type. Full explanation: OCA/server-tools#1642
guewen
force-pushed
the
10.0-sale-exception-no-side-effect
branch
from
302e368
to
aeb1f32
Compare
|
/ocabot merge |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
It looks like something changed on |
|
This PR has the |
|
It looks like something changed on |
|
Congratulations, your PR was merged at c8a9171. Thanks a lot for contributing to OCA. ❤️ |
The method called by 'sale_check_exception' has a side effect, it writes
on 'exception.rule' + on the Many2many relation between it and
sale.order(.line). When decorated by @api.constrains, any error during
the method will be caught and re-raised as "ValidationError".
This part of code is very prone to concurrent updates as 2 sales having
the same exception will both write on the same 'exception.rule'.
A concurrent update (OperationalError) is re-raised as ValidationError,
and then is not retried properly.
Calling the same method in create/write has the same effect than
@api.constrains without shadowing the exception type.
Full explanation:
OCA/server-tools#1642