Page MenuHomeSoftware Heritage
Differential D8218

feat: Make the Gogs lister incremental
ClosedPublic
Actions

Authored by KShivendu on Aug 9 2022, 7:45 AM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T1721: Implementation of Gogs Lister
Commits
rDLS6a53a6ad06d5: feat: Make the Gogs lister incremental
Summary

Make the Gogs lister incremental

Related to T1721

Diff Detail

Repository
rDLS Listers
Branch
feat/gogs-incremental-lister
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 30758
Build 48093: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 48092: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
170 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.gogs.tests.test_lister::test_gogs_full_listing
swh_scheduler = <swh.scheduler.backend.SchedulerBackend object at 0x7f593429f940> requests_mock = <requests_mock.mocker.Mocker object at 0x7f593429fb00> mocker = <pytest_mock.plugin.MockerFixture object at 0x7f593429f438>
2 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.arch.lister::swh.lister.arch.lister.size_to_bytes
2,007 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.arch.tests.test_lister::test_arch_lister
1,004 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.arch.tests.test_tasks::test_arch_lister
3,641 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.lister.arch.tests.test_tasks::test_arch_ping
View Full Test Results (1 Failed · 225 Passed)

Event Timeline

KShivendu created this revision.Aug 9 2022, 7:45 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptAug 9 2022, 7:45 AM
swh-public-ci added a comment.Aug 9 2022, 7:50 AM

Build is green

Patch application report for D8218 (id=29633)

Rebasing onto d51bce0a1c...

Current branch diff-target is up to date.
Changes applied before test
commit bb14166804858e0a0f1afcaa25d33c66403f8815
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:13:32 2022 +0530

    feat: Make the Gogs lister incremental

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

Harbormaster completed remote builds in B30701: Diff 29633.Aug 9 2022, 7:50 AM
KShivendu requested review of this revision.Aug 9 2022, 7:50 AM
KShivendu updated this revision to Diff 29634.Aug 9 2022, 8:07 AM
  • test: Improve variable names and comments in gogs lister tests
swh-public-ci added a comment.Aug 9 2022, 8:12 AM

Build is green

Patch application report for D8218 (id=29634)

Rebasing onto d51bce0a1c...

Current branch diff-target is up to date.
Changes applied before test
commit 642286955d9183eae1858ee4f4d9ad206d633918
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:36:53 2022 +0530

    test: Improve variable names and comments in gogs lister tests

commit bb14166804858e0a0f1afcaa25d33c66403f8815
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:13:32 2022 +0530

    feat: Make the Gogs lister incremental

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

Harbormaster completed remote builds in B30702: Diff 29634.Aug 9 2022, 8:12 AM
KShivendu mentioned this in T1721: Implementation of Gogs Lister.Aug 9 2022, 8:16 AM
vlorentz added a subscriber: vlorentz.Aug 9 2022, 3:12 PM
vlorentz added inline comments.
swh/lister/gogs/lister.py
141

not covered?

KShivendu updated this revision to Diff 29703.EditedAug 11 2022, 6:24 AM

test: Add test for incremental gogs lister and improve existing tests

  • Add test for incremental listing in gogs
  • Use 3 mocked pages instead of 2 in existing tests
  • In test_gogs_list_http_error, successfully parse 1 page before throwing HTTPError
  • Minor improvement in variables names (P1, P2, P3)
swh-public-ci added a comment.Aug 11 2022, 6:29 AM

Build is green

Patch application report for D8218 (id=29703)

Rebasing onto cee6bcb514...

First, rewinding head to replay your work on top of it...
Applying: feat: Make the Gogs lister incremental
Applying: test: Improve variable names and comments in gogs lister tests
Applying: test: Add test for incremental gogs lister and improve existing tests
Changes applied before test
commit b2557d51dce0981cdbc405d466324e99076cfd73
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Thu Aug 11 09:51:24 2022 +0530

    test: Add test for incremental gogs lister and improve existing tests

commit 1e4556b621f0b35b692f168995c38558430a31b7
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:36:53 2022 +0530

    test: Improve variable names and comments in gogs lister tests

commit 3a7288a149195d3be35f42dc2163cbf4b75c1680
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:13:32 2022 +0530

    feat: Make the Gogs lister incremental

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

Harbormaster completed remote builds in B30757: Diff 29703.Aug 11 2022, 6:29 AM
KShivendu updated this revision to Diff 29704.Aug 11 2022, 6:52 AM

fix: Improvements for edge cases and coverage

  • state.last_seen_next_link should be None if last page is encountered (no next link)
  • assert page.repos is not None in get_origins_from_page
  • Improve comments and formatting
swh-public-ci added a comment.Aug 11 2022, 6:56 AM

Build has FAILED

Patch application report for D8218 (id=29704)

Rebasing onto cee6bcb514...

First, rewinding head to replay your work on top of it...
Applying: feat: Make the Gogs lister incremental
Applying: test: Improve variable names and comments in gogs lister tests
Applying: test: Add test for incremental gogs lister and improve existing tests
Applying: fix: Improvements for edge cases and coverage
Changes applied before test
commit 549190d9035b556954478f145849d05f31924cb5
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Thu Aug 11 10:17:13 2022 +0530

    fix: Improvements for edge cases and coverage
    
    - state.last_seen_next_link should be None if last page is encountered
      (no next link)
    - `assert page.repos is not None` in get_origins_from_page
    - Improve comments and formatting

commit 590dd1a36165b8eed65d035897a88f3496c783a9
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Thu Aug 11 09:51:24 2022 +0530

    test: Add test for incremental gogs lister and improve existing tests

commit 5d83c3f66f8fc03edb30b99db86682e87e561787
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:36:53 2022 +0530

    test: Improve variable names and comments in gogs lister tests

commit e43e17f9712a943a9d7457f00fdef90172ada4ea
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:13:32 2022 +0530

    feat: Make the Gogs lister incremental

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

Harbormaster failed remote builds in B30758: Diff 29704!Aug 11 2022, 6:56 AM
KShivendu updated this revision to Diff 29705.Aug 11 2022, 7:28 AM

test: fix test failures

  • make last_seen_next_link = None if the last page is encountered
  • in test_gogs_incremental_lister, throw 400 in the 1st listing attempt and succeed in the 2nd one.
swh-public-ci added a comment.Aug 11 2022, 7:32 AM

Build is green

Patch application report for D8218 (id=29705)

Rebasing onto cee6bcb514...

First, rewinding head to replay your work on top of it...
Applying: feat: Make the Gogs lister incremental
Changes applied before test
commit 6a49b9dfddc78e13bc3e3629005db93dd6dd4a40
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:13:32 2022 +0530

    feat: Make the Gogs lister incremental

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

Harbormaster completed remote builds in B30759: Diff 29705.Aug 11 2022, 7:32 AM
vlorentz added inline comments.Aug 11 2022, 10:56 AM
swh/lister/gogs/tests/test_lister.py
214–217

Does it really mock the first listing attempt? I would expect the second request (with P2) not to have a Link header, so the lister wouldn't send the third request (with P3).

KShivendu added inline comments.Aug 11 2022, 2:18 PM
swh/lister/gogs/tests/test_lister.py
214–217

I would expect the second request (with P2) not to have a Link header

If P2 doesn't provide a "next" link header then the lister will throw an error because of assert len(response.links) > 0, "API changed: no Link header found" (same happens for Gitea lister)

What is the difference between a normal lister and an incremental one?
It is just that it restarts itself from the last state stored in the scheduler on running lister.run()

KShivendu added inline comments.Aug 11 2022, 2:32 PM
swh/lister/gogs/tests/test_lister.py
214–217

*If P2 doesn't provide a "next" link header

But, if it does provide a link header without a "next" link, then the first attempt will stop at page 2 and last_seen_next_link will be None. Now when lister.run() is used for the second time, it will recrawl both the pages again.

In the current test, the first lister fails and last_seen_next_link points to page 3. Hence in the second attempt, the lister only crawls that. Isn't this the expected behavior? Am I missing something?

vlorentz added inline comments.Aug 11 2022, 2:45 PM
swh/lister/gogs/tests/test_lister.py
214–217

It doesn't make sense for the last page to link to another page that doesn't exist.

I didn't try with Gogs as their API requires authentication, but with Gitea:

$ curl -i https://try.gitea.io/api/v1/repos/search/\?limit\=3 | grep "^link"
link: <https://try.gitea.io/api/v1/repos/search/?limit=3&page=2>; rel="next",<https://try.gitea.io/api/v1/repos/search/?limit=3&page=1753>; rel="last"

$ curl -i https://try.gitea.io/api/v1/repos/search/\?limit\=3\&page\=1753 | grep "^link"
link: <https://try.gitea.io/api/v1/repos/search/?limit=3&page=1>; rel="first",<https://try.gitea.io/api/v1/repos/search/?limit=3&page=1752>; rel="prev"

So there is indeed a link header, but no next link.

KShivendu added a subscriber: ardumont.Aug 12 2022, 1:11 PM
KShivendu added inline comments.
swh/lister/gogs/tests/test_lister.py
214–217

It doesn't make sense for the last page to link to another page that doesn't exist.

Actually, there are instances like T4423 where certain pages cannot be crawled because of some fatal repos. I've created an issue in the upstream but we haven't received any update from the Gogs maintainers.

I proposed T1721#88903 to fix this. You can read @ardumont's opinion on this in T1721#88915

vlorentz added inline comments.Aug 12 2022, 2:12 PM
swh/lister/gogs/tests/test_lister.py
214–217

but that's not what this test does. This test is written as a "normal" listing run and ends with a 400. T4423 is a bug, which is not normal, and ends with a 500.

KShivendu updated this revision to Diff 29729.EditedAug 16 2022, 5:07 AM
  • Introduce last_seen_repo_id in GogsListerState to properly set the value of self.updated in`finalize()`
  • Update incremental listing test to test happy flow (instead of the buggy T4423)
swh-public-ci added a comment.Aug 16 2022, 5:11 AM

Build is green

Patch application report for D8218 (id=29729)

Rebasing onto cee6bcb514...

First, rewinding head to replay your work on top of it...
Applying: feat: Make the Gogs lister incremental
Applying: feat: Introduce last_seen_repo_id in GogsListerState and update incremental listing test
Changes applied before test
commit 577d6104d6d67eec60d9661ca33dcc63609a9b9a
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Mon Aug 15 16:24:22 2022 +0530

    feat: Introduce last_seen_repo_id in GogsListerState and update incremental listing test

commit 5464574bba36bfa0e5f317b2bbd5bb3f75c9bc4f
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:13:32 2022 +0530

    feat: Make the Gogs lister incremental

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

Harbormaster completed remote builds in B30780: Diff 29729.Aug 16 2022, 5:11 AM
vlorentz added a comment.Aug 16 2022, 12:47 PM

Please add a test for when a new page (P4) is added after P3

KShivendu updated this revision to Diff 29772.Aug 17 2022, 10:08 AM
  • Introduce 4th page in incremental listing test
  • Improve comments
swh-public-ci added a comment.Aug 17 2022, 10:12 AM

Build is green

Patch application report for D8218 (id=29772)

Rebasing onto cee6bcb514...

First, rewinding head to replay your work on top of it...
Applying: feat: Make the Gogs lister incremental
Applying: feat: Introduce last_seen_repo_id in GogsListerState and update incremental listing test
Applying: test: Introduce 4th page and update the incremental listing test accordingly
Changes applied before test
commit a4dc4f9ec4348d379efca1e65bd8a522d44e1b21
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Wed Aug 17 13:10:19 2022 +0530

    test: Introduce 4th page and update the incremental listing test accordingly

commit 64fb84ed6716325c55c1fcb1bae57ce9a0b24c9f
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Mon Aug 15 16:24:22 2022 +0530

    feat: Introduce last_seen_repo_id in GogsListerState and update incremental listing test

commit a93300c88d7b0fa7b5a26f0df05d7c09c884a421
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:13:32 2022 +0530

    feat: Make the Gogs lister incremental

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

Harbormaster completed remote builds in B30824: Diff 29772.Aug 17 2022, 10:12 AM
vlorentz accepted this revision.Aug 17 2022, 11:30 AM

Thanks!

This revision is now accepted and ready to land.Aug 17 2022, 11:30 AM
KShivendu updated this revision to Diff 29778.Aug 17 2022, 11:32 AM

Squash commits and rebase

swh-public-ci added a comment.Aug 17 2022, 11:37 AM

Build is green

Patch application report for D8218 (id=29778)

Rebasing onto cee6bcb514...

Current branch diff-target is up to date.
Changes applied before test
commit 6a53a6ad06d5677d246732a8525fc931f2a2c7b1
Author: KShivendu <shivendu@iitbhilai.ac.in>
Date:   Tue Aug 9 11:13:32 2022 +0530

    feat: Make the Gogs lister incremental

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

Harbormaster completed remote builds in B30830: Diff 29778.Aug 17 2022, 11:37 AM
Closed by commit rDLS6a53a6ad06d5: feat: Make the Gogs lister incremental (authored by KShivendu). · Explain WhyAug 17 2022, 11:38 AM
This revision was automatically updated to reflect the committed changes.
KShivendu added a commit: rDLS6a53a6ad06d5: feat: Make the Gogs lister incremental.

Revision Contents
Changeset List

PathSize
swh/
lister/
gogs/
87 lines
tests/
data/
https_try.gogs.io/
168 lines
210 lines

Diff 29704

View Options

swh/lister/gogs/lister.py

Loading...
View Options

swh/lister/gogs/tests/data/https_try.gogs.io/repos_page3

Loading...
View Options

swh/lister/gogs/tests/test_lister.py

Loading...
About · Developer information · Legal