Page MenuHomeSoftware Heritage
Differential D5120

Deduplicate code between CoreSWHID, QualifiedSWHID, and ExtendedSWHID
ClosedPublic
Actions

Authored by vlorentz on Feb 19 2021, 4:07 PM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Commits
rDMOD9bcc88469c1d: Deduplicate code between CoreSWHID, QualifiedSWHID, and ExtendedSWHID
Summary

by making them all derive from an abstract class.

Depends on D5118.

Diff Detail

Repository
rDMOD Data model
Branch
base-swhid
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19395
Build 30079: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 30078: arc lint + arc unit

Event Timeline

vlorentz created this revision.Feb 19 2021, 4:07 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 19 2021, 4:07 PM
swh-public-ci added a comment.Feb 19 2021, 4:08 PM

Build is green

Patch application report for D5120 (id=18310)

Could not rebase; Attempt merge onto 758eb885d3...

Updating 758eb88..d46c31e
Fast-forward
 swh/model/identifiers.py            | 427 ++++++++++++++++++++++++++++--
 swh/model/tests/test_identifiers.py | 508 ++++++++++++++++++++++++++++++++++--
 tox.ini                             |   1 +
 3 files changed, 897 insertions(+), 39 deletions(-)
Changes applied before test
commit d46c31eb5422d78e480593d47074f5405da405bc
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 16:06:47 2021 +0100

    Deduplicate code between CoreSWHID, QualifiedSWHID, and ExtendedSWHID
    
    by making them all derive from an abstract class.

commit 36bfa7835736a9e3352657435e83234ef0ed6387
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 14:18:45 2021 +0100

    Add new class ExtendedSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the last one.
    
    It also removes "ori" as a valid object type for CoreSWHID and
    QualifiedSWHID, as it now only belongs in ExtendedSWHID.

commit 992376597d6f44dfe78c4eef40bf651fe355f0fd
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 13:58:03 2021 +0100

    Use dict instead of temporary SWHID when parsing {Core,Qualified}SWHID.
    
    It is cleaner, avoids warnings, and will be needed when introducing
    ExtendedSWHID in a future commit.

commit 8e917597dbb22054ada06387a0501452d8862d34
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 10:56:29 2021 +0100

    QualifiedSWHID: Replace the 'qualifiers' dict with statically defined attributes
    
    And store their parsed values (CoreSWHID, tuple of ints, etc.) instead of string.

commit eba8d84de660e2b3d7df304fa32da7404fb9f6bb
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 18 13:09:21 2021 +0100

    Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the second one

commit 690b7f824f55bc65fbec21b29714926476038abb
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 16 13:33:13 2021 +0100

    Add new class QualifiedSWHID to replace SWHID, and deprecate the latter.
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    Since migrating from SWHID will break existing code, this commit uses
    the opportunity to modernize it a little, ie.:
    
    * `keyword`-only constructor, to get rid of the hacky default values for
      `object_type` and `object_id`
    * enum instead of strings for the object type
    * `bytes` instead of an hex string for the object id
    * rename `metadata` to `qualifiers`

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

Harbormaster completed remote builds in B19395: Diff 18310.Feb 19 2021, 4:08 PM
vlorentz requested review of this revision.Feb 19 2021, 4:08 PM
vlorentz updated this revision to Diff 18311.Feb 19 2021, 4:09 PM

more dedup + fix error msg

swh-public-ci added a comment.Feb 19 2021, 4:10 PM

Build is green

Patch application report for D5120 (id=18311)

Could not rebase; Attempt merge onto 758eb885d3...

Updating 758eb88..bd84ec2
Fast-forward
 swh/model/identifiers.py            | 410 +++++++++++++++++++++++++++--
 swh/model/tests/test_identifiers.py | 508 ++++++++++++++++++++++++++++++++++--
 tox.ini                             |   1 +
 3 files changed, 880 insertions(+), 39 deletions(-)
Changes applied before test
commit bd84ec2860e2c8a16f804259c3fc9daa9cc6bd09
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 16:06:47 2021 +0100

    Deduplicate code between CoreSWHID, QualifiedSWHID, and ExtendedSWHID
    
    by making them all derive from an abstract class.

commit 36bfa7835736a9e3352657435e83234ef0ed6387
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 14:18:45 2021 +0100

    Add new class ExtendedSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the last one.
    
    It also removes "ori" as a valid object type for CoreSWHID and
    QualifiedSWHID, as it now only belongs in ExtendedSWHID.

commit 992376597d6f44dfe78c4eef40bf651fe355f0fd
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 13:58:03 2021 +0100

    Use dict instead of temporary SWHID when parsing {Core,Qualified}SWHID.
    
    It is cleaner, avoids warnings, and will be needed when introducing
    ExtendedSWHID in a future commit.

commit 8e917597dbb22054ada06387a0501452d8862d34
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 10:56:29 2021 +0100

    QualifiedSWHID: Replace the 'qualifiers' dict with statically defined attributes
    
    And store their parsed values (CoreSWHID, tuple of ints, etc.) instead of string.

commit eba8d84de660e2b3d7df304fa32da7404fb9f6bb
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 18 13:09:21 2021 +0100

    Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the second one

commit 690b7f824f55bc65fbec21b29714926476038abb
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 16 13:33:13 2021 +0100

    Add new class QualifiedSWHID to replace SWHID, and deprecate the latter.
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    Since migrating from SWHID will break existing code, this commit uses
    the opportunity to modernize it a little, ie.:
    
    * `keyword`-only constructor, to get rid of the hacky default values for
      `object_type` and `object_id`
    * enum instead of strings for the object type
    * `bytes` instead of an hex string for the object id
    * rename `metadata` to `qualifiers`

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

Harbormaster completed remote builds in B19396: Diff 18311.Feb 19 2021, 4:10 PM
vlorentz added a child revision: D5121: Deduplicate parsing/unparsing tests of the new SWHID classes.Feb 19 2021, 4:38 PM
olasd accepted this revision.Feb 23 2021, 11:27 AM
olasd added a subscriber: olasd.

Nice.

I guess mypy properly checks that the type of the <SWHIDClass>.object_type attribute matches the type parameter of the _BaseSWHID base class, and we don't need to add a test for that?

swh/model/identifiers.py
814
This revision is now accepted and ready to land.Feb 23 2021, 11:27 AM
vlorentz added a comment.Feb 23 2021, 1:28 PM
In D5120#129677, @olasd wrote:

I guess mypy properly checks that the type of the <SWHIDClass>.object_type attribute matches the type parameter of the _BaseSWHID base class, and we don't need to add a test for that?

I think so, yes. It feels overkill anyway

swh/model/identifiers.py
814

Nope, parts["qualifiers"] is an empty dict

vlorentz updated this revision to Diff 18342.Feb 23 2021, 1:48 PM

rebase

swh-public-ci added a comment.Feb 23 2021, 1:50 PM

Build is green

Patch application report for D5120 (id=18342)

Could not rebase; Attempt merge onto 758eb885d3...

Updating 758eb88..9bcc884
Fast-forward
 swh/model/identifiers.py            | 410 ++++++++++++++++++++++++++--
 swh/model/tests/test_identifiers.py | 514 ++++++++++++++++++++++++++++++++++--
 tox.ini                             |   1 +
 3 files changed, 886 insertions(+), 39 deletions(-)
Changes applied before test
commit 9bcc88469c1dd3080014dc4d05177724c2c9332d
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 16:06:47 2021 +0100

    Deduplicate code between CoreSWHID, QualifiedSWHID, and ExtendedSWHID
    
    by making them all derive from an abstract class.

commit d4b20dcdc2795d544461ea469e4156968f9e2cda
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 14:18:45 2021 +0100

    Add new class ExtendedSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the last one.
    
    It also removes "ori" as a valid object type for CoreSWHID and
    QualifiedSWHID, as it now only belongs in ExtendedSWHID.

commit 992376597d6f44dfe78c4eef40bf651fe355f0fd
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 13:58:03 2021 +0100

    Use dict instead of temporary SWHID when parsing {Core,Qualified}SWHID.
    
    It is cleaner, avoids warnings, and will be needed when introducing
    ExtendedSWHID in a future commit.

commit 8e917597dbb22054ada06387a0501452d8862d34
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Feb 19 10:56:29 2021 +0100

    QualifiedSWHID: Replace the 'qualifiers' dict with statically defined attributes
    
    And store their parsed values (CoreSWHID, tuple of ints, etc.) instead of string.

commit eba8d84de660e2b3d7df304fa32da7404fb9f6bb
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 18 13:09:21 2021 +0100

    Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the second one

commit 690b7f824f55bc65fbec21b29714926476038abb
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 16 13:33:13 2021 +0100

    Add new class QualifiedSWHID to replace SWHID, and deprecate the latter.
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    Since migrating from SWHID will break existing code, this commit uses
    the opportunity to modernize it a little, ie.:
    
    * `keyword`-only constructor, to get rid of the hacky default values for
      `object_type` and `object_id`
    * enum instead of strings for the object type
    * `bytes` instead of an hex string for the object id
    * rename `metadata` to `qualifiers`

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

Harbormaster completed remote builds in B19422: Diff 18342.Feb 23 2021, 1:50 PM
Closed by commit rDMOD9bcc88469c1d: Deduplicate code between CoreSWHID, QualifiedSWHID, and ExtendedSWHID (authored by vlorentz). · Explain WhyFeb 25 2021, 3:29 PM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDMOD9bcc88469c1d: Deduplicate code between CoreSWHID, QualifiedSWHID, and ExtendedSWHID.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
model/
162 lines98%
Loading...

Diff 18310

View Options

swh/model/identifiers.py

Loading...
About · Developer information · Legal