Page MenuHomeSoftware Heritage

client: add `visits` method to get the list of an origin visits
Needs ReviewPublic

Authored by lewo on Wed, Mar 25, 6:07 PM.

Details

Reviewers
zack
ardumont
Group Reviewers
Reviewers
Summary

A test is added to ensure the pagination and the data typing are
working as expected.

Diff Detail

Repository
rDWCLI Web client
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11592
Build 17577: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 17576: arc lint + arc unit

Event Timeline

lewo created this revision.Wed, Mar 25, 6:07 PM
olasd added a subscriber: olasd.Thu, Apr 2, 11:40 AM
olasd added inline comments.
swh/web/client/client.py
49

We use origin_visit in most places, so I guess this should use that name as well.

364

Should have a trailing slash.

swh/web/client/tests/gen-api-data.sh
33

If you add the trailing slash you shouldn't need to follow redirects.

lewo updated this revision to Diff 10488.Thu, Apr 2, 3:13 PM
lewo marked 3 inline comments as done.

@olasd I addressed all of your comments. Thanks for your review.

Build is green

Patch application report for D2881 (id=10488)

Rebasing onto 492be567e2...

Current branch diff-target is up to date.
Changes applied before test
commit 151b77349d34ead96b6667e472761b57e0454f3c
Author: Antoine Eiche <lewo@abesis.fr>
Date:   Wed Mar 25 17:52:19 2020 +0100

    client: add `visits` method to get the list of an origin visits
    
    A test is added to ensure the pagination and the data typing are
    working as expected.

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

zack requested changes to this revision.Thu, Apr 2, 5:10 PM

LGTM, just minor docstring/comment issue, then we're good to go

thanks !

Do not forget to add yourself to the top-level CONTRIBUTORS file

swh/web/client/client.py
343

maybe "origin: the URL of a software origin"

344

just "number of visits to list", for consistency with other docstrings

swh/web/client/tests/conftest.py
24–25

this is no longer "the only URL" :-)

can you fix this comment and the one you copied this from, to simply state why each URL needs monkey patching?

This revision now requires changes to proceed.Thu, Apr 2, 5:10 PM
ardumont added inline comments.
swh/web/client/client.py
352

indentation ;)

lewo updated this revision to Diff 10500.Thu, Apr 2, 6:23 PM

Thanks @zack @ardumont. I addressed your comments.

Build is green

Patch application report for D2881 (id=10500)

Rebasing onto 492be567e2...

Current branch diff-target is up to date.
Changes applied before test
commit 8d47566785824aa219ddb44e51e7b364b2fca54c
Author: Antoine Eiche <lewo@abesis.fr>
Date:   Wed Mar 25 17:52:19 2020 +0100

    client: add `visits` method to get the list of an origin visits
    
    A test is added to ensure the pagination and the data typing are
    working as expected.

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

ardumont accepted this revision.Thu, Apr 2, 6:55 PM

fine for me and thanks ;)