-
-
Notifications
You must be signed in to change notification settings - Fork 191
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
12.0 mig sale_financial_risk #14
Conversation
70075a3
to
3769244
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new field is very heavy to be compute. Can we light this up a bit with some read_group
operations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new field seems expensive, but at least it's stored, so it shouldn't be much of a problem.
The read_group
isn't a panacea: it saves some Python loops, but at the cost of increasing SQL queries (because you can't benefit of caching and prefetching). I think we can defer that "fix" until we see it's actually a problem.
Code OK, just some little obvious remarks for a little better performance.
3769244
to
f39b951
Compare
@carlosdauden your last call about the method's performance. @ernestotejeda please cover with tests all the code for not dropping coverage. |
Done |
…invoices) Company currency
[IMP] sale_financial_risk: Simplify code (Remove no directly related invoices)
cbe22da
to
12824cb
Compare
Cc @Tecnativa