Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] product: clear caches less aggressively #32802

Open
wants to merge 1 commit into
base: 12.0
from

Conversation

Projects
None yet
3 participants
@seb-odoo
Copy link
Contributor

commented Apr 18, 2019

Follow up of 5b32f1b

We now only invalidate what has to be invalidated.

Also ormcache does not expect a recordset as parameter.

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@seb-odoo seb-odoo added the RD label Apr 18, 2019

@seb-odoo seb-odoo self-assigned this Apr 18, 2019

@robodoo robodoo added the seen 🙂 label Apr 18, 2019

@seb-odoo seb-odoo force-pushed the odoo-dev:12.0-cache-seb branch 3 times, most recently from 2fe63a1 to b9f9c34 Apr 18, 2019

@seb-odoo seb-odoo added the 12.0 label Apr 18, 2019

@seb-odoo seb-odoo force-pushed the odoo-dev:12.0-cache-seb branch 2 times, most recently from bd63e6c to 3bcc2c4 Apr 18, 2019

@robodoo robodoo added the CI 🤖 label Apr 18, 2019

@seb-odoo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

@rco-odoo the cache fixes in 12.0 we discussed the last time. Could you review it please?

@seb-odoo seb-odoo requested a review from rco-odoo Apr 23, 2019

@rco-odoo
Copy link
Member

left a comment

Please don't modify code without a reason.
If you fix a bug, please add a test to reproduce it!

Show resolved Hide resolved addons/product/models/product_template.py Outdated

@seb-odoo seb-odoo force-pushed the odoo-dev:12.0-cache-seb branch 2 times, most recently from 6b10d5f to 3af7799 May 8, 2019

[FIX] product: clear caches less aggressively
Follow up of 5b32f1b

We now only invalidate what has to be invalidated.

Also ormcache does not expect a recordset as parameter, as the parameters
specify how to build a key for the cache, therefore they must be hashable.

@seb-odoo seb-odoo force-pushed the odoo-dev:12.0-cache-seb branch from 3af7799 to fec49e7 May 8, 2019

@robodoo robodoo removed the CI 🤖 label May 8, 2019

@seb-odoo

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2019

@rco-odoo what part of the code here do you consider modified without a reason? For me the reason is just performance, as it invalidated more cache than what was necessary. There are already tests covering the need to invalidate itself.

If you are not confident with the invalidate changes in stable, we could merge just the ormcache key change here and do the invalidation in master. Let me know.

@robodoo robodoo added the CI 🤖 label May 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.