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

[IMP] account_tax_balance: Optimized computation of move type #643

Conversation

yajo
Copy link
Member

@yajo yajo commented Dec 12, 2019

Reduced complexity based on read_group. This method was prefetching a lot and doing many slow operations.

Executing in a real database for 197361 recs. took 14 minutes before and 1:36 minutes now.

@Tecnativa TT18838

@yajo yajo force-pushed the 9.0-account_tax_balance-move_type_compute_optimized branch from fd24356 to 01c0ea1 Compare December 12, 2019 10:32
@yajo yajo marked this pull request as ready for review December 12, 2019 10:33
@yajo
Copy link
Member Author

yajo commented Dec 12, 2019

What happens with travis? 😵

@pedrobaeza pedrobaeza added this to the 9.0 milestone Dec 12, 2019
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

I have fixed .travis.yml in main branch

account_tax_balance/models/account_move.py Outdated Show resolved Hide resolved
account_tax_balance/models/account_move.py Outdated Show resolved Hide resolved
@yajo yajo force-pushed the 9.0-account_tax_balance-move_type_compute_optimized branch from 01c0ea1 to e6b134c Compare December 13, 2019 10:43
@yajo
Copy link
Member Author

yajo commented Dec 13, 2019

All done.

@pedrobaeza
Copy link
Member

If you rebase, we get a green CI for better display

@yajo yajo force-pushed the 9.0-account_tax_balance-move_type_compute_optimized branch from e6b134c to 57b2bca Compare December 16, 2019 09:11
@yajo
Copy link
Member Author

yajo commented Dec 16, 2019

Rebased.

@yajo yajo force-pushed the 9.0-account_tax_balance-move_type_compute_optimized branch from 57b2bca to 39142d7 Compare December 16, 2019 09:38
@carlosdauden
Copy link

Fix tests

Reduced complexity based on `read_group`. This method was prefetching a lot and doing many slow operations.

Executing in a real database for 197361 recs. took 14 minutes before and 1:36 minutes now.

@Tecnativa TT18838
@yajo yajo force-pushed the 9.0-account_tax_balance-move_type_compute_optimized branch from 39142d7 to f3ddb4f Compare December 16, 2019 12:49
@yajo
Copy link
Member Author

yajo commented Dec 16, 2019

Done, thanks.

@pedrobaeza
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 9.0-ocabot-merge-pr-643-by-pedrobaeza-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Dec 16, 2019
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot merged commit f3ddb4f into OCA:9.0 Dec 16, 2019
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza deleted the 9.0-account_tax_balance-move_type_compute_optimized branch December 16, 2019 15:16
@pedrobaeza
Copy link
Member

I think this improvement is as well valid for 10.0, 11.0, 12.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.

None yet

4 participants