Page MenuHomeSoftware Heritage

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

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

Details

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

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.

rebase + fix docstring + remove useless test

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.

olasd requested changes to this revision.Tue, Feb 23, 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.Tue, Feb 23, 9:50 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

This revision is now accepted and ready to land.Tue, Feb 23, 11:31 AM
ardumont added a subscriber: ardumont.

same as nico, accepted through the extra diff