Change Details

I thought having easily fix the issue reported in T1997 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 [[ https://archive.softwareheritage.org/browse/search/?q=forge.softwareheritage.org&with_visit&with_content for instance | here ]] 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 T997 (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.

I thought having easily fix the issue reported in T1997 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 [[ https://archive.softwareheritage.org/browse/search/?q=forge.softwareheritage.org&with_visit&with_content for instance | here ]] 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 T1997 (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.

I thought having easily fix the issue reported in T1997 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 [[ https://archive.softwareheritage.org/browse/search/?q=forge.softwareheritage.org&with_visit&with_content for instance | here ]] 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 T1997 (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.