Page MenuHomeSoftware Heritage

deposit_list: Only enrich deposit metadata in page
ClosedPublic

Authored by ardumont on Mar 7 2022, 6:04 PM.

Details

Summary

Nonetheless, it's actually swh-web side that most of the work needs to happen to make
the moderation view fast.

Related to T4020

Test Plan

tox

Diff Detail

Repository
rDDEP Push deposit
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27326
Build 42744: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 42743: arc lint + arc unit

Event Timeline

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 response

See https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/795/ for more details.

I don't understand why moving this code enriches the output data.

Could you also add a test for this?

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.

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?

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.

Oh, I see. Then you need to replace "Actually" with "Only" in your diff/commit title

This revision is now accepted and ready to land.Mar 8 2022, 3:57 PM
ardumont retitled this revision from deposit_list: Actually enrich deposit metadata in page to deposit_list: Only enrich deposit metadata in page.Mar 8 2022, 4:02 PM

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 ;)"

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 response

See https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/798/ for more details.