Skip to content

[12.0][FIX] company_today: Execute every 15 min, remove doall #265

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

Open
wants to merge 2 commits into
base: 12.0
Choose a base branch
from

Conversation

carmenbianca
Copy link
Member

Signed-off-by: Carmen Bianca Bakker [email protected]

Description

Odoo task (if applicable)

Checklist before approval

  • Tests are present (or not needed).
  • Credits/copyright have been changed correctly.
  • Change log snippet is present.
  • (If a new module) Moving this to OCA has been considered.

@codecov-commenter
Copy link

Codecov Report

Merging #265 (ec993b2) into 12.0 (0f05213) will increase coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             12.0     #265      +/-   ##
==========================================
+ Coverage   70.00%   70.09%   +0.09%     
==========================================
  Files         124      126       +2     
  Lines        1637     1642       +5     
  Branches      316      317       +1     
==========================================
+ Hits         1146     1151       +5     
  Misses        458      458              
  Partials       33       33              
Impacted Files Coverage Δ
partner_no_unique_bank/models/res_partner_bank.py 100.00% <0.00%> (ø)
partner_no_unique_bank/models/__init__.py 100.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@huguesdk huguesdk left a comment

Choose a reason for hiding this comment

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

i think that once per a few hours is enough (for our use case at least). for other use cases, once per hour is ok, i think. there is the (rather unlikely, but still possible) case where odoo is not running at the cron time, but in that case, today will only be off during one hour maximum. i think that this is a reasonable limitation (to put in the readme), compared to computing things over and over uselessly: even if the date does not change, all dependent computed values will be recomputed (since odoo does not check whether the value has changed or not), unless .cron_update_today() first checks whether the value needs to be updated.

@carmenbianca
Copy link
Member Author

for other use cases, once per hour is ok, i think

I am inclined to think not. Nepal has an offset of UTC+5:45.

unless .cron_update_today() first checks whether the value needs to be updated.

I knew I forgot something. I'll do that.

All fields are recomputed when the dependent field is written to, even
if the value of the dependent field did not change. By reducing the
amount of writes to the field, we reduce the amount of recomputation.

Signed-off-by: Carmen Bianca Bakker <[email protected]>
@carmenbianca carmenbianca requested a review from huguesdk October 31, 2022 07:26
@robinkeunen
Copy link
Member

robinkeunen commented Oct 31, 2022

for other use cases, once per hour is ok, i think

I am inclined to think not. Nepal has an offset of UTC+5:45.

Also the town of Eucla (UTC+08:45) at the border of West Australia (UTC+08:00) and Southern Australia (UTC+09:45). Apparently, it was more convenient to relay telegraph messages. Maintained for retro-compatibility I guess...

@robinkeunen
Copy link
Member

@carmenbianca should we merge or close ?

@huguesdk huguesdk changed the title [FIX] company_today: Execute every 15 min, remove doall [12.0][FIX] company_today: Execute every 15 min, remove doall Apr 16, 2025
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.

5 participants