Differential D1929

cgit: rewrite the CGit lister
ClosedPublic
Actions

Authored by douardda on Aug 30 2019, 6:03 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Commits

rDLS3816b4d3bf9c: cgit: rewrite the CGit lister

Summary

Simplify the code:

do only inherit from ListerBase
implement HTTP queries directly using requests
get rid of convoluted code

Make the origin_url gathered from the git repo's "project" page instead of
building it from the 'url_prefix' hack. Now, the lister WILL make substancially
more requests, since it will make one request per listed git repo, but
the provided origin_url should be pretty reliable now.

When several url are provided as clonable URLs, choose the http/https one first,
otherwise, choose the first one of the list.

Add proper tests for the cgit lister.

Also, get rid of the 'time_updated' column in the model.

Depends on D1928

Diff Detail

Repository

rDLS Listers

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

douardda created this revision.Aug 30 2019, 6:03 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptAug 30 2019, 6:03 PM

douardda added a child revision: D1930: core: make it possible to specify the api_baseurl init argument in override_config.Aug 30 2019, 6:03 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/314/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/314/console

Harbormaster failed remote builds in B7541: Diff 6485!Aug 30 2019, 6:13 PM

This one, i'll have a look on monday ;)

Don't this need a rebase (to make jenkins happy)?
The jenkins failure seems to be out of its depth.

I just have a minor nitpick about the tests with multiple pages.

Otherwise, this looks good, thx.

swh/lister/cgit/lister.py
131	I guess the html_url, full_name, etc... and other unpopulated field dbs are defaulting to null values (so it does not break ;). And are also what needs to be further dealt with via T1978
swh/lister/cgit/tests/test_lister.py
72	Don't we want to check a tad the urls as you did in the other tests?

In D1929#44784, @ardumont wrote:

Don't this need a rebase (to make jenkins happy)?
The jenkins failure seems to be out of its depth.

I thinks this needs a fix in cli.py (which is undone/deleted in D1504 but meh). Let me check that.

douardda added inline comments.Sep 2 2019, 10:58 AM

swh/lister/cgit/lister.py
131	That's the idea yes.
swh/lister/cgit/tests/test_lister.py
72	As you wish, master ;-)

Btw, i think we can make this related to T1861 (bullet 1. as implementation).

nahimilega added a subscriber: nahimilega.Sep 2 2019, 11:14 AM

nahimilega added inline comments.

swh/lister/cgit/lister.py
13–14	You could add a docstring to the class. Something like this https://forge.softwareheritage.org/source/swh-lister/browse/master/swh/lister/packagist/lister.py$0-15 IIRC, we decided to make a docstring for the lister class which shows their output. I forgot to create a task regarding this(my bad)