Skip to content

Phabricator: Fix loading tasks creation

Antoine Lambert requested to merge generated-differential-D1982-source into generated-differential-D1982-target

I thought having easily fix the issue reported in #1997 (closed) but while trying to implement a test to validate the changes, I noticed there was a major issue in the current lister implementation.

Turns out except the first listed repository (whose index is retrieved through a bootstrap process) all other listed ones were filtered out. Consequently, no entries in the listers database and no scheduler loading tasks were created when listing a Phabricator instance.

I now better understand why the Phabricator instances we listed so far in production still do not have any visit (see [here](https://archive.softwareheritage.org/browse/search/?q=forge.softwareheritage.org&with_visit&with_content for instance) for instance).

This critical issue is due to the max_index member overriding of the Phabricator lister when it bootstraps to get the first repository index. Unfortunately, the tests were passing as the conditions were still respected (but with only one repository ingested in listers database).

That diff fixes that nasty bug and also the issue observed in #1997 (closed) (less critical as it will only appear when listing repositories when the max_bound parameter is provided to the run method of the lister, the fact that we encountered it is due to the bug described above). I also fix some typos to make my git pre-commit hook happy.

Test Plan

Tests implementation have been improved and new ones have been added to capture the observed issues.

Migrated from D1982 (view on Phabricator)

Merge request reports