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.

Sign up for GitHub

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

Jump to bottom

perf: reduce number of queries to validate selling price #26225

Merged
merged 5 commits into from
Aug 24, 2021
Merged

perf: reduce number of queries to validate selling price #26225

merged 5 commits into from
Aug 24, 2021

Conversation

pruthvi145
Copy link
Contributor

@pruthvi145 pruthvi145 commented Jun 26, 2021 •
edited

~58% reduction in SellingController.validate_selling_price

Changes Made

  • Optimize the query call from linear to constant.
  • Use table tabBin instead of tabStock Ledger Entry for fetching last_valuation_rate.
  • Format the error message.

Screenshots

Before

Screenshot-2021-06-28-114423

After

Screenshot-2021-06-28-114519

@pruthvi145 pruthvi145 force-pushed the perf-validate-selling-price branch 2 times, most recently from 6b2ab7b to 3269a3c Compare June 26, 2021 13:03
@sagarvora sagarvora force-pushed the perf-validate-selling-price branch from fdfa2ed to 8c1616a Compare June 28, 2021 06:23
@sagarvora sagarvora marked this pull request as ready for review June 28, 2021 06:25
@sagarvora sagarvora force-pushed the perf-validate-selling-price branch from 8c1616a to 79cc302 Compare June 28, 2021 06:25
ankush
ankush reviewed Jun 28, 2021
View reviewed changes
erpnext/controllers/selling_controller.py Outdated Show resolved Hide resolved
erpnext/controllers/selling_controller.py Show resolved Hide resolved
erpnext/controllers/selling_controller.py Outdated Show resolved Hide resolved
@sagarvora sagarvora requested a review from ankush July 8, 2021 07:12
@sagarvora sagarvora added the squash Meant to tell reviewers that this PR should be squashed into a single commit while merging. label Jul 8, 2021
@ankush ankush requested review from nextchamp-saqib and deepeshgarg007 and removed request for ankush July 15, 2021 12:19
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 42.928% when pulling a89c520 on resilient-tech:perf-validate-selling-price into 6442b5d on frappe:develop.

@stale stale bot added the inactive label Jul 30, 2021
@frappe frappe deleted a comment from stale bot Jul 30, 2021
@pruthvi145 pruthvi145 requested a review from ankush July 31, 2021 03:28
@nabinhait nabinhait merged commit 7c957d7 into frappe:develop Aug 24, 2021
frappe-pr-bot pushed a commit to frappe-pr-bot/erpnext that referenced this pull request Aug 24, 2021
@pruthvi145 @frappe-bot
perf: reduce number of queries to validate selling price (frappe#26225)
b11d60c 
* perf: reduce number of queries to validate selling price

* fix: improved flow and formatting

* fix: improve condition and use of `as_dict`

Co-authored-by: Sagar Vora <sagar@resilient.tech>
(cherry picked from commit 7c957d7)
@frappe-pr-bot frappe-pr-bot mentioned this pull request Aug 24, 2021
Merged
AfshanKhan pushed a commit that referenced this pull request Aug 24, 2021
@frappe-pr-bot @pruthvi145
perf: reduce number of queries to validate selling price (#26225) (#2…
8474961 
…7119)

* perf: reduce number of queries to validate selling price

* fix: improved flow and formatting

* fix: improve condition and use of `as_dict`

Co-authored-by: Sagar Vora <sagar@resilient.tech>
(cherry picked from commit 7c957d7)

Co-authored-by: Pruthvi Patel <pruthvipatel145@gmail.com>
asoral pushed a commit to asoral/erpnext that referenced this pull request Nov 12, 2021
@pruthvi145 @sagarvora
perf: reduce number of queries to validate selling price (frappe#26225)
042d565 
* perf: reduce number of queries to validate selling price

* fix: improved flow and formatting

* fix: improve condition and use of `as_dict`

Co-authored-by: Sagar Vora <sagar@resilient.tech>
@deepeshgarg007 deepeshgarg007 mentioned this pull request Jul 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sagarvora sagarvora Awaiting requested review from sagarvora

@nextchamp-saqib nextchamp-saqib Awaiting requested review from nextchamp-saqib

@deepeshgarg007 deepeshgarg007 Awaiting requested review from deepeshgarg007 deepeshgarg007 is a code owner

@ankush ankush Awaiting requested review from ankush

Assignees
No one assigned
Labels
backport version-13-hotfix squash Meant to tell reviewers that this PR should be squashed into a single commit while merging.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@pruthvi145 @coveralls @ankush @sagarvora @nabinhait