Remove debian folder committed by mistake

• add a test to check lister instantiation with configuration file

In D4961#124944, @ardumont wrote:

Well, we can't really do that, we currently totally inhibit the lister.from_configfile call.

Ah right, I guess we should add the test in test_lister.py then. I will update D4962 then.

yes, indeed and that's what i'm currently doing ;)

(great news about launchpad ;)

In D4961#124940, @ardumont wrote:

load_from_envvar = mocker.patch("swh.lister.pattern.load_from_envvar")
load_from_envvar.return_value = {"credentials": {}}

Well, we can't really do that, we currently totally inhibit the lister.from_configfile call.

Note that this lister seems to need some writing improvments though.
It seemed to have flushed the writing only at the end of the listing.
If that's the real behavior (i'll need to check), that won't bode well for relatively high dimensioned instance like the cgit eclispe instance for example.

In D4961#124922, @ardumont wrote:

Could you also update the celery tasks tests by mocking credentials configuration coming from configuration file ?

good idea, i'll check that.

---

Independently from this diff but still i think possibly related to stateless lister in general, you might be interested by [1]

[1] https://forge.softwareheritage.org/T2998#57500

LGTM. Other ported listers might have the same issue so I guess they should be fixed too.

Legacy option should be removed soon so no tests needed here ;-)

This would be cleaner to process the url in the lister constructor instead (same as in debian one).

In D4955#124737, @ardumont wrote:

@anlambert, thanks, jsyk, i'm working on fixing failing test in the chroot debian in the end goal to deploy the listers in staging.

Looks good !

In D4954#124699, @ardumont wrote:

Less code to modify and easier to read and understand the intended behavior from my point of view.

maybe so.

One pro I see for your implementation is if at some point the main loop (get_pages)
fails to fetch something (which we don't currently expect to see the assert added
here), we'll have more information available in the log instead of an AssertionError.
We'll indeed have an actual HttpError with the status code and everything in the
stacktrace.

All in all, i think you are right ;)

In D4954#124697, @ardumont wrote:

You should rather catch the requests.exceptions.HTTPError originally raised by _get_and_parse method.

picture me curious, why?

(it's missing the eventual logging.warning instruction but its use is debatable as well ;)

You should rather catch the requests.exceptions.HTTPError originally raised by _get_and_parse method.

Looks good to me !

Thanks for tackling this !

Rebase

Rebase and remove conftest.py file

Thanks !

Looks good but the commit message could be more detailed and rephrased to something like:

Looks good to me ! I added a comment regarding further improvements that could be added to the lister but not related to that diff.

Rebase

Following URLs are now redirected to the desired page:

• https://softwareheritage.org/schema/2018/deposit
• https://softwareheritage.org/schema/2018/deposit/

Rebase

Update: Add test for parse_packaged_date and fix implementation.

Update: Address some @tenma comments

Looks good !

On a related note, I found this list of Community-Hosted GitLab Instances. Most of them have public access and could be added to the set of Gitlab instances listed by Software Heritage.

Sorry I accepted the diff before seeing you used Basic Auth which I think is not working.

I do not see any section regarding basic HTTP authentication for API requests in Gitlab API doc. Are you sure it is working ?

You should add a test to check the last_update field value in scheduler database is not None.

In D4940#124126, @swh-public-ci wrote:

Build has FAILED

Patch application report for D4940 (id=17591)

Rebasing onto b6a69b2ed9...

Current branch diff-target is up to date.

Changes applied before test

commit b66ffc6c441dba8d01325bf8c648ba667f14798c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Jan 25 18:15:21 2021 +0100

    gitlab: make url mandatory and add type

commit cc9cccdcc284738a3e20ea1a7a71f8c814c2ace6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Jan 25 15:32:49 2021 +0100

    gitlab: Support authentication
    
    Related to T2987

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

Remove debian/conftest.py modified by black from the commit.

Looks good to me !

Looks good to me !

Rebase

Great !

Rebase

Looks good to me !

Looks good to me !

Looks good to me !