Page MenuHomeSoftware Heritage

origin: Migrate use to storage.origin_list instead of origin_get_range
ClosedPublic

Authored by ardumont on Fri, Jul 31, 4:19 PM.

Details

Summary

The origin-get-range endpoint will be dropped from the storage.

Note: This will actually make it possible to use the webapp with cassandra now for that part
(cassandra storage does not expose the origin-get-range endpoint).

Related to T645
Related to T2214

Test Plan

tox

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Fri, Jul 31, 4:19 PM
ardumont edited the summary of this revision. (Show Details)Fri, Jul 31, 4:25 PM
vlorentz requested changes to this revision.Fri, Jul 31, 4:30 PM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/web/api/views/origin.py
85–98

change the name of the parameter.

also raise an error if the old name is passed, so clients who still use it get a proper error

101

here too

swh/web/common/service.py
251–252

-> PagedResultInfo[OriginInfo]

This revision now requires changes to proceed.Fri, Jul 31, 4:30 PM
ardumont added inline comments.Fri, Jul 31, 4:30 PM
swh/web/api/views/origin.py
86

Kept origin-from and origin-count which are exposed as is.

swh/web/common/service.py
251–252

yes, fixed locally already, i waited for the build to finish.

Build is green

Patch application report for D3675 (id=12931)

Rebasing onto ddfb658d25...

Current branch diff-target is up to date.
Changes applied before test
commit a28dd1bd54421e7b1e317b406f3c332bdf532c9e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 31 16:17:53 2020 +0200

    origin: Migrate use to storage.origin_list instead of origin_get_range
    
    Related to T645

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

vlorentz added inline comments.Fri, Jul 31, 4:35 PM
swh/web/api/views/origin.py
86

I know, but we should change the name of origin_from because its semantics changed.

ardumont updated this revision to Diff 12933.Fri, Jul 31, 4:47 PM

Adapt according to review:

  • rename query parameters
  • fail with a 400 and an explanatory message when old query parameters are used
  • Add tests around those use cases
swh/web/api/views/origin.py
86

oh yeah, said that prior to you asking me to change

I agree, was unsure on how to proceed ;)

Build is green

Patch application report for D3675 (id=12933)

Rebasing onto ddfb658d25...

Current branch diff-target is up to date.
Changes applied before test
commit 7f930c261f4a14a813e39c49a89280e2f8b58cbf
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 31 16:17:53 2020 +0200

    origin: Migrate use to storage.origin_list instead of origin_get_range
    
    Related to T645

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

I don't see any reason to change the name of origin_count, I think we should keep it

i misunderstood

ardumont updated this revision to Diff 12935.Fri, Jul 31, 5:22 PM

Keep origin_count parameter name

vlorentz added inline comments.Fri, Jul 31, 5:28 PM
swh/web/api/views/origin.py
68

forgot to undo this change

Build is green

Patch application report for D3675 (id=12935)

Rebasing onto ddfb658d25...

Current branch diff-target is up to date.
Changes applied before test
commit de3d682e268d00c0cdc5059ff04d713b58796f63
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 31 16:17:53 2020 +0200

    origin: Migrate use to storage.origin_list instead of origin_get_range
    
    Related to T645

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

ardumont updated this revision to Diff 12936.Fri, Jul 31, 5:38 PM

Revert limit mention in docstring

Build has FAILED

Patch application report for D3675 (id=12936)

Rebasing onto ddfb658d25...

Current branch diff-target is up to date.
Changes applied before test
commit e543cfdf56c19755b0d82f15b9e9a154ecd10f9f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 31 16:17:53 2020 +0200

    origin: Migrate use to storage.origin_list instead of origin_get_range
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/292/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/292/console

Build is green

Patch application report for D3675 (id=12936)

Rebasing onto ddfb658d25...

Current branch diff-target is up to date.
Changes applied before test
commit e543cfdf56c19755b0d82f15b9e9a154ecd10f9f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 31 16:17:53 2020 +0200

    origin: Migrate use to storage.origin_list instead of origin_get_range
    
    Related to T645

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

vlorentz added inline comments.Fri, Jul 31, 10:52 PM
swh/web/api/views/origin.py
82–83

this one too (sorry)

88

The error message should say to use the Link header. page_token is not part of the API and users shouldn't are about it, as explained in the docstring

ardumont added inline comments.Sat, Aug 1, 9:25 AM
swh/web/api/views/origin.py
82–83

don't be, that's fine

it's on me, i looked but fail to see it ;)

88

right.

ardumont updated this revision to Diff 12960.Sat, Aug 1, 9:25 AM

Adapt according to last good remarks:

  • fix missing revert on docstring
  • Explicit the 400 error message when api is misused

Build is green

Patch application report for D3675 (id=12960)

Rebasing onto ddfb658d25...

Current branch diff-target is up to date.
Changes applied before test
commit b19fb3f661fe5b4cd786dd3af68fa6e883a9a10d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 31 16:17:53 2020 +0200

    origin: Migrate use to storage.origin_list instead of origin_get_range
    
    Related to T645

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

vlorentz accepted this revision.Mon, Aug 3, 9:07 AM
This revision is now accepted and ready to land.Mon, Aug 3, 9:07 AM