Page MenuHomeSoftware Heritage
Differential D4962

launchpad: Reimplement lister using new Lister API
ClosedPublic
Actions

Authored by anlambert on Jan 28 2021, 2:02 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T2992: Port launchpad lister to the new Lister API
Commits
rDLSf86200470025: launchpad: Reimplement lister using new Lister API
Summary

Port launchpad lister to the swh.lister.pattern.Lister API.

Last update date of each listed git repositories is now sent to the scheduler.

The lister can work in incremental mode, only modified repositories since
the last listing operation will be returned in that case.

Closes T2992

Diff Detail

Repository
rDLS Listers
Branch
launchpad-lister-new-api
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18851
Build 29204: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 29203: arc lint + arc unit

Event Timeline

anlambert created this revision.Jan 28 2021, 2:02 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJan 28 2021, 2:02 PM
swh-public-ci added a comment.Jan 28 2021, 2:06 PM

Build is green

Patch application report for D4962 (id=17704)

Rebasing onto 72be074a79...

Current branch diff-target is up to date.
Changes applied before test
commit 58c0b9c10f4b94702cdac0c994bd332ae06634ec
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Jan 27 18:58:54 2021 +0100

    launchpad: Reimplement lister using new Lister API
    
    Port launchpad lister to the swh.lister.pattern.Lister API.
    
    Last update date of each listed git repositories is now sent to the scheduler.
    
    The lister can work in incremental mode, only modified repositories since
    the last listing operation will be returned in that case.
    
    Closes T2992

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

Harbormaster completed remote builds in B18845: Diff 17704.Jan 28 2021, 2:06 PM
anlambert requested review of this revision.Jan 28 2021, 2:06 PM
anlambert mentioned this in D4961: Make stateless lister constructors compatible with credentials.Jan 28 2021, 2:13 PM
anlambert updated this revision to Diff 17707.Jan 28 2021, 2:37 PM

Update:

  • add a test to check lister instantiation with configuration file
  • remove test_get_lister restriction for launchpad now everything is properly mocked
swh-public-ci added a comment.Jan 28 2021, 2:41 PM

Build is green

Patch application report for D4962 (id=17707)

Rebasing onto 72be074a79...

Current branch diff-target is up to date.
Changes applied before test
commit 49b63e04f039be2c89eb5efc3f3f3ab8120a930b
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Jan 27 18:58:54 2021 +0100

    launchpad: Reimplement lister using new Lister API
    
    Port launchpad lister to the swh.lister.pattern.Lister API.
    
    Last update date of each listed git repositories is now sent to the scheduler.
    
    The lister can work in incremental mode, only modified repositories since
    the last listing operation will be returned in that case.
    
    Closes T2992

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

Harbormaster completed remote builds in B18848: Diff 17707.Jan 28 2021, 2:41 PM
ardumont added a subscriber: ardumont.Jan 28 2021, 2:48 PM
ardumont added inline comments.
debian/changelog
676 ↗(On Diff #17707)

too much info in your commits ^
I guess you tested the debian build works but you committed too much in here ;)

swh/lister/launchpad/__init__.py
11

to answer our previous discussion/question, we can drop it apparently, nothing is complaining so far, well lister wise at least.

anlambert added inline comments.Jan 28 2021, 2:54 PM
debian/changelog
676 ↗(On Diff #17707)

oh jeez, thanks !

anlambert updated this revision to Diff 17710.Jan 28 2021, 2:54 PM

Remove debian folder committed by mistake

anlambert added inline comments.Jan 28 2021, 2:56 PM
swh/lister/launchpad/__init__.py
11

yes, we can remove the remaining one in ported listers safely.

swh-public-ci added a comment.Jan 28 2021, 2:58 PM

Build is green

Patch application report for D4962 (id=17710)

Rebasing onto ae17b6b9a0...

First, rewinding head to replay your work on top of it...
Applying: launchpad: Reimplement lister using new Lister API
Changes applied before test
commit 7adee7239f6ab8f47d0d41a91caf379ac7b90cc2
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Jan 27 18:58:54 2021 +0100

    launchpad: Reimplement lister using new Lister API
    
    Port launchpad lister to the swh.lister.pattern.Lister API.
    
    Last update date of each listed git repositories is now sent to the scheduler.
    
    The lister can work in incremental mode, only modified repositories since
    the last listing operation will be returned in that case.
    
    Closes T2992

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

Harbormaster completed remote builds in B18851: Diff 17710.Jan 28 2021, 2:58 PM
ardumont added a comment.Jan 28 2021, 3:11 PM

lgtm so far (did not read the test)

couple of questions inline.

swh/lister/launchpad/lister.py
71

Should the computation about updating/restoring the state within those methods?
What's the point of the finalize method then?

Also shouldn't there be comparison between date to keep only the most recent of the modified date?

106

Why not constrain the iso8601 conversion back and forth here?

anlambert added inline comments.Jan 28 2021, 3:15 PM
swh/lister/launchpad/lister.py
71

Those methods are only there to serialize/deserialize the lister state in order to be able to sent it through RPC.

106

Because we want to manipulate datetime objects while the lister runs.

ardumont added inline comments.Jan 28 2021, 3:17 PM
swh/lister/launchpad/lister.py
106

right, i misread that part.

ardumont accepted this revision.Jan 28 2021, 3:21 PM

yep, looks good to me.

Thanks.

swh/lister/launchpad/lister.py
71

indeed, i mixed the 2 dates notions we have.

The one used for the incremental nature of the lister (pagination "index" date used to store the state, what we are using) and the "last_update" for one repo to list.

moving along ;)

Thanks.

This revision is now accepted and ready to land.Jan 28 2021, 3:21 PM
anlambert updated this revision to Diff 17714.Jan 28 2021, 3:23 PM

Rebase

anlambert added a comment.Jan 28 2021, 3:24 PM
In D4962#125005, @ardumont wrote:

yep, looks good to me.

Thanks.

Two remaining ones to port: gnu and packagist. Then we will be able to drop the dependency to SQLAlchemy.

swh-public-ci added a comment.Jan 28 2021, 3:26 PM

Build is green

Patch application report for D4962 (id=17714)

Rebasing onto ae17b6b9a0...

Current branch diff-target is up to date.
Changes applied before test
commit f862004700259234a690779b48dd19c37c93d5a0
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Jan 27 18:58:54 2021 +0100

    launchpad: Reimplement lister using new Lister API
    
    Port launchpad lister to the swh.lister.pattern.Lister API.
    
    Last update date of each listed git repositories is now sent to the scheduler.
    
    The lister can work in incremental mode, only modified repositories since
    the last listing operation will be returned in that case.
    
    Closes T2992

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

Harbormaster completed remote builds in B18854: Diff 17714.Jan 28 2021, 3:26 PM
Closed by commit rDLSf86200470025: launchpad: Reimplement lister using new Lister API (authored by anlambert). · Explain WhyJan 28 2021, 3:28 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDLSf86200470025: launchpad: Reimplement lister using new Lister API.
anlambert mentioned this in D4964: launchpad/tasks: Fix ping task function name.Jan 28 2021, 4:12 PM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
6 lines--
swh/
lister/
launchpad/
4 lines91%
Loading...
207 lines100%
Loading...
--
41 lines100%
Loading...
tests/
data/
107 lines--
107 lines--
--
--
--
164 lines100%
Loading...
52 lines100%
Loading...
tests/
3 lines100%
Loading...

Diff 17710

View Options

mypy.ini

Loading...
View Options

swh/lister/launchpad/__init__.py

Loading...
View Options

swh/lister/launchpad/lister.py

Loading...
View Options

swh/lister/launchpad/models.py

Loading...
View Options

swh/lister/launchpad/tasks.py

Loading...
View Options

swh/lister/launchpad/tests/data/launchpad_response1.json

Loading...
View Options

swh/lister/launchpad/tests/data/launchpad_response2.json

Loading...
View Options

swh/lister/launchpad/tests/data/response0.json

Loading...
View Options

swh/lister/launchpad/tests/data/response1.json

Loading...
View Options

swh/lister/launchpad/tests/data/response2.json

Loading...
View Options

swh/lister/launchpad/tests/test_lister.py

Loading...
View Options

swh/lister/launchpad/tests/test_tasks.py

Loading...
View Options

swh/lister/tests/test_cli.py

Loading...
About · Developer information · Legal