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

perf: reduce DB calls in frappe.client.get (and other changes) #17665

Merged
merged 1 commit into from Aug 2, 2022

Conversation

sagarvora
Copy link
Member

@sagarvora sagarvora commented Jul 28, 2022

DB calls reduced:

  • frappe.db.get_value to get name from filters
  • frappe.get_doc repeated when returning 🤦🏼

We lose the ability to have a special error message in case of filters, but I don't think that's a big deal.


There is one small change in client.get_single_value as well, we're no longer creating a variable just to return it.

@sagarvora sagarvora requested a review from a team as a code owner July 28, 2022 20:00
@sagarvora sagarvora requested review from surajshetty3416 and removed request for a team July 28, 2022 20:00
@sagarvora sagarvora changed the title perf: reduce DB calls in frappe.client.get perf: reduce DB calls in frappe.client.get (and other changes) Jul 28, 2022
@sagarvora sagarvora force-pushed the simplify-client.get branch 2 times, most recently from ed77572 to 9427121 Compare July 30, 2022 04:21
@codecov
Copy link

codecov bot commented Jul 30, 2022

Codecov Report

Merging #17665 (49daa30) into develop (e72a02e) will decrease coverage by 1.81%.
The diff coverage is 75.00%.

@@             Coverage Diff             @@
##           develop   #17665      +/-   ##
===========================================
- Coverage    59.61%   57.79%   -1.82%     
===========================================
  Files          759      759              
  Lines        68530    68524       -6     
  Branches      6044     6044              
===========================================
- Hits         40853    39606    -1247     
- Misses       24209    25450    +1241     
  Partials      3468     3468              
Flag Coverage Δ
server 61.96% <75.00%> (-2.88%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@ankush ankush added the backport version-14-hotfix backport to version 14 label Aug 2, 2022
@ankush ankush merged commit ebb0cd1 into frappe:develop Aug 2, 2022
@ankush ankush deleted the simplify-client.get branch August 2, 2022 10:08
mergify bot pushed a commit that referenced this pull request Aug 2, 2022
(cherry picked from commit ebb0cd1)

# Conflicts:
#	frappe/client.py
mergify bot pushed a commit that referenced this pull request Aug 2, 2022
ankush pushed a commit that referenced this pull request Aug 2, 2022
(cherry picked from commit ebb0cd1)

Co-authored-by: Sagar Vora <sagar@resilient.tech>
ankush added a commit that referenced this pull request Aug 2, 2022
…kport #17665) (#17710)

* perf: reduce DB call in `frappe.client.get` (#17665)

(cherry picked from commit ebb0cd1)

# Conflicts:
#	frappe/client.py

* chore: conflicts

Co-authored-by: Sagar Vora <sagar@resilient.tech>
Co-authored-by: Ankush Menat <ankush@frappe.io>
frappe-pr-bot pushed a commit that referenced this pull request Aug 3, 2022
## [14.0.1](v14.0.0...v14.0.1) (2022-08-03)

### Bug Fixes

* Append to condition misleading message ([#17696](#17696)) ([#17709](#17709)) ([f05b517](f05b517))
* **global_search:** Trigger rebuilding on Custom Field's property change ([#17706](#17706)) ([a5d576f](a5d576f))
* max_positive_value for Integer types ([#17712](#17712)) ([#17714](#17714)) ([306af63](306af63))
* set /app as redirect for pageview home button ([#17715](#17715)) ([#17717](#17717)) ([594944a](594944a))
* translate each role ([6816f83](6816f83))
* use `throw` ([f7ce174](f7ce174))
* use warn util ([c66fa28](c66fa28))

### Performance Improvements

* reduce DB call in `frappe.client.get` ([#17665](#17665)) ([#17711](#17711)) ([1655fb3](1655fb3))
* specify reference doctype in filters ([#17704](#17704)) ([fc230b4](fc230b4))
developmentforpeople pushed a commit to developmentforpeople/frappe that referenced this pull request Aug 7, 2022
## [14.0.1](frappe/frappe@v14.0.0...v14.0.1) (2022-08-03)

### Bug Fixes

* Append to condition misleading message ([frappe#17696](frappe#17696)) ([frappe#17709](frappe#17709)) ([f05b517](frappe@f05b517))
* **global_search:** Trigger rebuilding on Custom Field's property change ([frappe#17706](frappe#17706)) ([a5d576f](frappe@a5d576f))
* max_positive_value for Integer types ([frappe#17712](frappe#17712)) ([frappe#17714](frappe#17714)) ([306af63](frappe@306af63))
* set /app as redirect for pageview home button ([frappe#17715](frappe#17715)) ([frappe#17717](frappe#17717)) ([594944a](frappe@594944a))
* translate each role ([6816f83](frappe@6816f83))
* use `throw` ([f7ce174](frappe@f7ce174))
* use warn util ([c66fa28](frappe@c66fa28))

### Performance Improvements

* reduce DB call in `frappe.client.get` ([frappe#17665](frappe#17665)) ([frappe#17711](frappe#17711)) ([1655fb3](frappe@1655fb3))
* specify reference doctype in filters ([frappe#17704](frappe#17704)) ([fc230b4](frappe@fc230b4))
frappe-pr-bot pushed a commit that referenced this pull request Aug 9, 2022
# [13.37.0](v13.36.3...v13.37.0) (2022-08-09)

### Bug Fixes

* allow system managers to toggle email queue ([c004846](c004846))
* allow to import time field ([#17677](#17677)) ([#17690](#17690)) ([59f58c7](59f58c7))
* check permission before sending email ([bb73014](bb73014))
* Dropdown selection list in Reports should be translatable ([#17679](#17679)) ([8c0e5ab](8c0e5ab))
* **global_search:** Trigger rebuilding on Custom Field's property change ([#17705](#17705)) ([b4ff5b2](b4ff5b2))
* max_positive_value for Integer types ([#17712](#17712)) ([#17713](#17713)) ([e173e64](e173e64))
* merge conflict ([2f71fb1](2f71fb1))
* restored patch and remove unused import ([b2232f1](b2232f1))
* return promise ([#17646](#17646)) ([#17666](#17666)) ([d931e26](d931e26))
* Show Report & Dashboard View for File Doctype (backport [#17688](#17688)) ([#17695](#17695)) ([ad09e87](ad09e87))
* version bump regex in automated release ([#17725](#17725)) ([#17726](#17726)) ([a4f682a](a4f682a))

### Features

* Role based permission for Dashboard Chart ([#17634](#17634)) ([#17655](#17655)) ([c3f45e7](c3f45e7))

### Performance Improvements

* reduce DB calls in `frappe.client.get` (and other changes) (backport [#17665](#17665)) ([#17710](#17710)) ([5fe4924](5fe4924))
* reduce DB calls made in `get_fetch_values` ([#17671](#17671)) ([#17738](#17738)) ([fc58a87](fc58a87))
* specify reference doctype in filters ([#17703](#17703)) ([88a59d8](88a59d8))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants