[ADD] invalid-commit: Add new check invalid-commit

[moylop260]

• Verify the guideline never-commit-the-transaction

[coveralls]

Coverage remained the same at 100.0% when pulling 4d4831c on vauxoo-dev:master-oca-add-sql-commit-moy into 6a8f696 on OCA:master.

[pedrobaeza]

👍

[luiseevaquer]

👍

[guewen]

Commit could still be valid such as in cron tasks or for performance reasons.
This is also noticed by the guidelines in:

All cr.commit() calls outside of the server framework from now on must have an explicit comment explaining why they are absolutely necessary, why they are indeed correct, and why they do not break the transactions. Otherwise they can and will be removed!

[moylop260]

@guewen
You are right in some cases we need cr.commit
First step: Could you use cr.savepoint? No
Second step: Could you use a duplicated cursor to isolate the current transaction (More info here)? No

with openerp.api.Environment.manage():
    with openerp.registry(self.env.cr.dbname).cursor() as new_cr:
        # Create a new environment with new cursor database
        new_env = api.Environment(new_cr, self.env.uid, self.env.context)
        # with_env replace original env for this method
        self.with_env(new_env).write({'name': 'hello'})  # isolated transaction to commit
        new_env.cr.commit()  # Don't show a invalid-commit in this case
    # You don't need close your cr because is closed when finish "with"
# You don't need clear caches because is cleared when finish "with"

Third step: Then use cr.commit, but add an explicit comment explaining... in this comment you can add # pylint: disable=invalid-commit to bypass this check.

[guewen]

Third step: Then use cr.commit, but add an explicit comment explaining... in this comment you can add # pylint: disable=invalid-commit to bypass this check.

ok noted, thanks
👍

[oscarolar]

👍

[moylop260]

Somebody, could be merged it?