Skip to content

[10.0] base_exception: Remove side effect from api.constrains - #1644

Merged
OCA-git-bot merged 1 commit into
OCA:10.0from
guewen:10.0-base-exception-no-constrains-side-effect
Oct 14, 2019
Merged

OCA-git-bot merged 1 commit into
OCA:10.0from
guewen:10.0-base-exception-no-constrains-side-effect

Conversation

@guewen

@guewen guewen commented Aug 13, 2019

Copy link
Copy Markdown
Member

In the documentation.

The method called by '_check_exception' has a side effect, it writes
on 'exception.rule' + on the Many2many relation between it and
the related model (such as sale.order). 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:
#1642

In the documentation.

The method called by '_check_exception' has a side effect, it writes
on 'exception.rule' + on the Many2many relation between it and
the related model (such as sale.order). 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#1642
@guewen guewen changed the title base_exception: Remove side effect from api.constrains [10.0] base_exception: Remove side effect from api.constrains Aug 13, 2019

@florian-dacosta florian-dacosta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@guewen

guewen commented Oct 14, 2019

Copy link
Copy Markdown
Member Author

/ocabot merge

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 10.0-ocabot-merge-pr-1644-by-guewen-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 14, 2019
Signed-off-by guewen
@OCA-git-bot
OCA-git-bot merged commit 2750e15 into OCA:10.0 Oct 14, 2019
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at f472093. Thanks a lot for contributing to OCA. ❤️

SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (17.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants