Nonetheless, it's actually swh-web side that most of the work needs to happen to make
the moderation view fast.
Related to T4020
Details
Details
- Reviewers
vlorentz - Group Reviewers
Reviewers - Maniphest Tasks
- T4020: deposit moderation view is slow
- Commits
- rDDEPcac15113293a: deposit_list: Only enrich deposit metadata in page
tox
Diff Detail
Diff Detail
- Repository
- rDDEP Push deposit
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 27337 Build 42762: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 42761: arc lint + arc unit
Event Timeline
Comment Actions
Build is green
Patch application report for D7308 (id=26439)
Could not rebase; Attempt merge onto 5271378581...
Updating 52713785..bf1b7e7b Fast-forward swh/deposit/api/private/deposit_list.py | 43 +++++++++++++++++++++------------ swh/deposit/client.py | 9 ++++++- 2 files changed, 35 insertions(+), 17 deletions(-)
Changes applied before test
commit bf1b7e7bdc078cccada3142d9bfb6ceb015f779a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Mon Mar 7 18:01:24 2022 +0100
deposit_list: Actually enrich deposit metadata in page
Related to T4020
commit ddfeadcb62b67e214cd7f7aa99948a0e39bc7309
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Fri Feb 25 13:55:03 2022 +0100
swh.deposit.client: Fix unparsable error responseSee https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/795/ for more details.
Comment Actions
I don't understand why moving this code enriches the output data.
Could you also add a test for this?
Comment Actions
I don't understand why moving this code enriches the output data.
It's only doing the enriching part on the queryset for the page concerned. whereas the
old code resolved the queryset which are lazy by default to actuall enrich result. And
then pagination happened.
Could you also add a test for this?
it's already covered by existing test.
Comment Actions
I still don't get it.
Are you saying the old code did not enrich data at all? Then why did tests pass for the old code?
Comment Actions
I still don't get it.
It did.
But it resolved all deposits first and then for each of those deposits, did extra query to get the information it wants.
So now, it does that enrichment only when a page is required.
Comment Actions
Oh, I see. Then you need to replace "Actually" with "Only" in your diff/commit title
yeah, i said "actually" because i thought it was already the case... and then not.
So that's "please code, actually do what i meant earlier ;)"
Comment Actions
Build is green
Patch application report for D7308 (id=26452)
Could not rebase; Attempt merge onto 1fe5bf1913...
Updating 1fe5bf19..cac15113 Fast-forward swh/deposit/api/private/deposit_list.py | 43 +++++++++++++++++++++------------ swh/deposit/client.py | 9 ++++++- 2 files changed, 35 insertions(+), 17 deletions(-)
Changes applied before test
commit cac15113293ad5fc13a494737aaa152939cc79d6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Mon Mar 7 18:01:24 2022 +0100
deposit_list: Only enrich deposit metadata in page
Prior to this, this was fetching all deposits and then for each deposit, query further
information. Then return results and let the pagination happen.
This now keeps the queryset lazy, the pagination happens and when a page is requested,
this fetches further information on the subset required.
Related to T4020
commit 2c6e56caa062d24fb7648a23ca80790431cbaa63
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Fri Feb 25 13:55:03 2022 +0100
swh.deposit.client: Fix unparsable error responseSee https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/798/ for more details.