Page MenuHomeSoftware Heritage
Differential D7069

Add typing to revision_walker.py and make the state a dataclass
ClosedPublic
Actions

Authored by vlorentz on Feb 3 2022, 11:50 AM.

Details

Reviewers
douardda
Group Reviewers
Reviewers
Commits
rDSTO75a7f0934385: Add typing to revision_walker.py and make the state a dataclass

Diff Detail

Repository
rDSTO Storage manager
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26484
Build 41414: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 41413: arc lint + arc unit

Event Timeline

vlorentz created this revision.Feb 3 2022, 11:50 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 3 2022, 11:50 AM
swh-public-ci added a comment.Feb 3 2022, 12:11 PM

Build was aborted

Patch application report for D7069 (id=25654)

Rebasing onto 4544d7ca8e...

Current branch diff-target is up to date.
Changes applied before test
commit 7c9dae2beb61267952f6b9d322bc49ad89650199
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 3 11:50:25 2022 +0100

    Add typing to revision_walker.py.

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 3 2022, 12:11 PM
Harbormaster failed remote builds in B26484: Diff 25654!
vlorentz requested review of this revision.Feb 3 2022, 1:26 PM
swh-public-ci added a comment.Feb 3 2022, 1:47 PM

Build was aborted

Patch application report for D7069 (id=25654)

Rebasing onto 4544d7ca8e...

Current branch diff-target is up to date.
Changes applied before test
commit 7c9dae2beb61267952f6b9d322bc49ad89650199
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 3 11:50:25 2022 +0100

    Add typing to revision_walker.py.

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

swh-public-ci added a comment.Feb 3 2022, 3:08 PM

Build is green

Patch application report for D7069 (id=25654)

Rebasing onto 4544d7ca8e...

Current branch diff-target is up to date.
Changes applied before test
commit 7c9dae2beb61267952f6b9d322bc49ad89650199
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 3 11:50:25 2022 +0100

    Add typing to revision_walker.py.

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

Harbormaster completed remote builds in B26484: Diff 25654.Feb 3 2022, 3:08 PM
olasd added a subscriber: olasd.Feb 4 2022, 11:26 AM

I see you have to always ignore the typing when manipulating self._state.revs_to_visit, would it make more sense to make it an attribute of the child classes directly?

You would need to add a get_revs_to_visit()/set_revs_to_visit() method to be able to de/serialize it, but maybe the typing would be cleaner?

swh/storage/algos/revisions_walker.py
300

doubleplustype: ignore for more effect

douardda added a subscriber: douardda.Feb 4 2022, 11:27 AM

lgtm but as said on IRC, it's not just a type annotation diff, it does modify the code (introduce the State dataclass etc)

It could even be 2 git revisions TBH, but I would not veto this, as is, providing a better commit messsage.

vlorentz added a child revision: D7070: revision_walker: Add support for ignore_displayname..Feb 4 2022, 11:28 AM
vlorentz added a comment.Feb 4 2022, 11:32 AM
In D7069#184304, @olasd wrote:

I see you have to always ignore the typing when manipulating self._state.revs_to_visit, would it make more sense to make it an attribute of the child classes directly?

Not always, only in child classes that don't use the same type as the parent

You would need to add a get_revs_to_visit()/set_revs_to_visit() method to be able to de/serialize it, but maybe the typing would be cleaner?

Actually i'll just make it an Any

vlorentz updated this revision to Diff 25694.Feb 4 2022, 11:32 AM

remove 'type: ignore' and reword commit msg

swh-public-ci added a comment.Feb 4 2022, 11:34 AM

Build has FAILED

Patch application report for D7069 (id=25694)

Rebasing onto 4544d7ca8e...

Current branch diff-target is up to date.
Changes applied before test
commit 916de20a360e52cd90f9e5baee8f8743847b03f8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 3 11:50:25 2022 +0100

    Add typing to revision_walker.py and make the state a dataclass

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

Harbormaster failed remote builds in B26523: Diff 25694!Feb 4 2022, 11:34 AM
swh-public-ci added a comment.Feb 4 2022, 1:56 PM

Build has FAILED

Patch application report for D7069 (id=25694)

Rebasing onto 4544d7ca8e...

Current branch diff-target is up to date.
Changes applied before test
commit 916de20a360e52cd90f9e5baee8f8743847b03f8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 3 11:50:25 2022 +0100

    Add typing to revision_walker.py and make the state a dataclass

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

vlorentz retitled this revision from Add typing to revision_walker.py. to Add typing to revision_walker.py and make the state a dataclass.Feb 4 2022, 1:58 PM
vlorentz updated this revision to Diff 25707.Feb 4 2022, 3:02 PM

rebase

swh-public-ci added a comment.Feb 4 2022, 3:12 PM

Build is green

Patch application report for D7069 (id=25707)

Could not rebase; Attempt merge onto 4544d7ca8e...

Updating 4544d7ca..7c10f7a1
Fast-forward
 requirements-test.txt                 |   3 +-
 swh/storage/algos/revisions_walker.py | 147 ++++++++++++++++++----------------
 2 files changed, 79 insertions(+), 71 deletions(-)
Changes applied before test
commit 7c10f7a1cf57daee7b597d3ebd581d67a0db5b74
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 3 11:50:25 2022 +0100

    Add typing to revision_walker.py and make the state a dataclass

commit a3a63d8d409ec04d4f1eaf98ff9ef703c2957960
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 4 15:01:47 2022 +0100

    Require pytest to be <7.0.0

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

Harbormaster completed remote builds in B26536: Diff 25707.Feb 4 2022, 3:12 PM
douardda added inline comments.Feb 4 2022, 4:09 PM
swh/storage/algos/revisions_walker.py
197

shouldn't this be updated?

douardda accepted this revision.Feb 4 2022, 4:10 PM

probably need to update the docstring, otherwise ok

This revision is now accepted and ready to land.Feb 4 2022, 4:10 PM
vlorentz updated this revision to Diff 25716.Feb 4 2022, 4:14 PM

fix docstring

This revision was landed with ongoing or failed builds.Feb 4 2022, 4:15 PM
Closed by commit rDSTO75a7f0934385: Add typing to revision_walker.py and make the state a dataclass (authored by vlorentz). · Explain Why
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDSTO75a7f0934385: Add typing to revision_walker.py and make the state a dataclass.
swh-public-ci added a comment.Feb 4 2022, 4:25 PM

Build is green

Patch application report for D7069 (id=25716)

Could not rebase; Attempt merge onto 4544d7ca8e...

Updating 4544d7ca..75a7f093
Fast-forward
 requirements-test.txt                 |   3 +-
 swh/storage/algos/revisions_walker.py | 149 ++++++++++++++++++----------------
 2 files changed, 80 insertions(+), 72 deletions(-)
Changes applied before test
commit 75a7f09343853f63270e796c95b4ecf52d4984d5
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 3 11:50:25 2022 +0100

    Add typing to revision_walker.py and make the state a dataclass

commit a3a63d8d409ec04d4f1eaf98ff9ef703c2957960
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 4 15:01:47 2022 +0100

    Require pytest to be <7.0.0

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

Harbormaster completed remote builds in B26545: Diff 25716.Feb 4 2022, 4:25 PM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
storage/
algos/
149 lines72%
Loading...

Diff 25654

View Options

swh/storage/algos/revisions_walker.py

Loading...
About · Developer information · Legal