Page MenuHomeSoftware Heritage
Differential D5116

QualifiedSWHID: Replace the 'qualifiers' dict with statically defined attributes
ClosedPublic
Actions

Authored by vlorentz on Feb 19 2021, 10:56 AM.

Details

Reviewers
olasd
ardumont
Group Reviewers
Reviewers
Commits
rDMOD8e917597dbb2: QualifiedSWHID: Replace the 'qualifiers' dict with statically defined attributes
Summary

And store their parsed values (CoreSWHID, tuple of ints, etc.) instead of string.

Diff Detail

Repository
rDMOD Data model
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Feb 19 2021, 10:56 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 19 2021, 10:56 AM
swh-public-ci added a comment.Feb 19 2021, 10:58 AM

Build is green

Patch application report for D5116 (id=18293)

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

Updating 758eb88..1f488d9
Fast-forward
 swh/model/identifiers.py            | 306 +++++++++++++++++++++++++++-
 swh/model/tests/test_identifiers.py | 383 ++++++++++++++++++++++++++++++++++--
 tox.ini                             |   1 +
 3 files changed, 667 insertions(+), 23 deletions(-)
Changes applied before test
commit 1f488d9f90c68e1c7fbba0ea2a03d33a4193e933
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 ad46e9a6816fa5e3d0993deabf729fa299184944
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 50a4eaff5b29439d900ba128d93f0cb075ecc60c
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/243/ for more details.

Harbormaster completed remote builds in B19379: Diff 18293.Feb 19 2021, 10:58 AM
vlorentz requested review of this revision.Feb 19 2021, 10:58 AM
vlorentz updated this revision to Diff 18302.Feb 19 2021, 1:50 PM

rebase + fix docstring + remove useless test

swh-public-ci added a comment.Feb 19 2021, 1:54 PM

Build is green

Patch application report for D5116 (id=18302)

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

Updating 758eb88..8e91759
Fast-forward
 swh/model/identifiers.py            | 306 +++++++++++++++++++++++++++++-
 swh/model/tests/test_identifiers.py | 359 ++++++++++++++++++++++++++++++++++--
 tox.ini                             |   1 +
 3 files changed, 643 insertions(+), 23 deletions(-)
Changes applied before test
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/246/ for more details.

Harbormaster completed remote builds in B19388: Diff 18302.Feb 19 2021, 1:54 PM
vlorentz added a child revision: D5117: Use dict instead of temporary SWHID when parsing {Core,Qualified}SWHID..Feb 19 2021, 2:17 PM
vlorentz added a parent revision: D5107: Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID.Feb 19 2021, 2:17 PM
olasd requested changes to this revision.Feb 23 2021, 9:50 AM
olasd added a subscriber: olasd.

Looks good overall but a small issue has been found.

swh/model/identifiers.py
827

Instead of tuple(map()) you can probably write this in a way that doesn't need a type ignore.

957

Doesn't that return "1-None" for a single line?

This revision now requires changes to proceed.Feb 23 2021, 9:50 AM
olasd added a comment.EditedFeb 23 2021, 11:11 AM

Ah, one more change I think of when looking at D5118: please add a test to check that all values of the SWHID_QUALIFIERS set correspond to an attribute of QualifiedSWHID

vlorentz added a comment.Feb 23 2021, 11:23 AM

D5127

olasd accepted this revision.Feb 23 2021, 11:31 AM

via D5127

This revision is now accepted and ready to land.Feb 23 2021, 11:31 AM
ardumont accepted this revision.Feb 23 2021, 12:09 PM
ardumont added a subscriber: ardumont.

same as nico, accepted through the extra diff

Closed by commit rDMOD8e917597dbb2: QualifiedSWHID: Replace the 'qualifiers' dict with statically defined attributes (authored by vlorentz). · Explain WhyFeb 25 2021, 3:28 PM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDMOD8e917597dbb2: QualifiedSWHID: Replace the 'qualifiers' dict with statically defined attributes.

Revision Contents
Changeset List

PathSize
swh/
model/
104 lines
tests/
46 lines

Diff 18394

View Options

swh/model/identifiers.py

Loading...
View Options

swh/model/tests/test_identifiers.py

Loading...
About · Developer information · Legal