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.

Harbormaster completed remote builds in B27326: Diff 26439.Mar 7 2022, 6:10 PM

ardumont requested review of this revision.Mar 7 2022, 6:10 PM

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

vlorentz accepted this revision.Mar 8 2022, 3:57 PM

This revision is now accepted and ready to land.Mar 8 2022, 3:57 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 ;)"

Rebase

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.

Harbormaster completed remote builds in B27337: Diff 26452.Mar 8 2022, 4:21 PM

Closed by commit rDDEPcac15113293a: deposit_list: Only enrich deposit metadata in page (authored by ardumont). · Explain WhyMar 8 2022, 4:38 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDDEPcac15113293a: deposit_list: Only enrich deposit metadata in page.