Page MenuHomeSoftware Heritage
Differential D4461

identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid
ClosedPublic
Actions

Authored by ardumont on Nov 12 2020, 10:16 AM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T2769: Make function swh.model.identifiers.parse_swhid more strict
Commits
rDMODfb504b400ba9: identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid
Summary

Enfore checks on the qualifiers so invalid swhid are rejected.

Related to T2769
Depends on D4458

Test Plan

tox

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

ardumont created this revision.Nov 12 2020, 10:16 AM
ardumont edited the summary of this revision. (Show Details)Nov 12 2020, 10:16 AM
ardumont added a parent revision: D4458: model.identifiers: Improve error messages in case of invalid SWHIDs.
swh-public-ci added a comment.Nov 12 2020, 10:17 AM

Build is green

Patch application report for D4461 (id=15816)

Could not rebase; Attempt merge onto 4e3fdc062c...

Updating 4e3fdc0..870fbc7
Fast-forward
 swh/model/identifiers.py            | 67 ++++++++++++++++++++++++++++------
 swh/model/tests/test_identifiers.py | 73 +++++++++++++++++++++----------------
 2 files changed, 97 insertions(+), 43 deletions(-)
Changes applied before test
commit 870fbc70dda134b3265f20b2054a788012ad4c13
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Nov 12 10:08:45 2020 +0100

    identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid
    
    Related to T2769

commit 87071750c81642933a5c936b7282d6da04dddd2d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 18:21:01 2020 +0100

    model.identifiers: Improve error message by exposing the grammar
    
    Related to T2769

commit 47946aac0220fcfd55c7998908c71a6f402a5ddb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 16:11:32 2020 +0100

    test: Migrate parse_swhid test cases to pytest
    
    Related to T2769

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

Harbormaster completed remote builds in B17026: Diff 15816.Nov 12 2020, 10:18 AM
ardumont updated this revision to Diff 15819.Nov 12 2020, 10:20 AM
  • Fix noisy characters in noqa comments
  • add comments to explicit what's wrong with swhid
swh-public-ci added a comment.Nov 12 2020, 10:21 AM

Build is green

Patch application report for D4461 (id=15819)

Could not rebase; Attempt merge onto 4e3fdc062c...

Updating 4e3fdc0..49f3f5c
Fast-forward
 swh/model/identifiers.py            | 67 ++++++++++++++++++++++++++------
 swh/model/tests/test_identifiers.py | 77 ++++++++++++++++++++++---------------
 2 files changed, 101 insertions(+), 43 deletions(-)
Changes applied before test
commit 49f3f5c4253a27b14ecffe5755aa4895bfee85aa
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Nov 12 10:08:45 2020 +0100

    identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid
    
    Related to T2769

commit 87071750c81642933a5c936b7282d6da04dddd2d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 18:21:01 2020 +0100

    model.identifiers: Improve error message by exposing the grammar
    
    Related to T2769

commit 47946aac0220fcfd55c7998908c71a6f402a5ddb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 16:11:32 2020 +0100

    test: Migrate parse_swhid test cases to pytest
    
    Related to T2769

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

Harbormaster completed remote builds in B17029: Diff 15819.Nov 12 2020, 10:21 AM
ardumont updated this revision to Diff 15821.Nov 12 2020, 10:26 AM

Rebase

swh-public-ci added a comment.Nov 12 2020, 10:28 AM

Build is green

Patch application report for D4461 (id=15821)

Could not rebase; Attempt merge onto 4e3fdc062c...

Updating 4e3fdc0..b924c07
Fast-forward
 swh/model/identifiers.py            | 71 ++++++++++++++++++++++++++++------
 swh/model/tests/test_identifiers.py | 77 ++++++++++++++++++++++---------------
 2 files changed, 105 insertions(+), 43 deletions(-)
Changes applied before test
commit b924c077d8681cbc82cbd809e9fbc4a9eb29c6f2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Nov 12 10:08:45 2020 +0100

    identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid
    
    Related to T2769

commit cb11afe5c7fae1301a26a9b500ec2c2932170e5c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 18:21:01 2020 +0100

    model.identifiers: Improve error message by exposing the grammar
    
    Related to T2769

commit 47946aac0220fcfd55c7998908c71a6f402a5ddb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 16:11:32 2020 +0100

    test: Migrate parse_swhid test cases to pytest
    
    Related to T2769

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

Harbormaster completed remote builds in B17031: Diff 15821.Nov 12 2020, 10:28 AM
ardumont added a child revision: D4462: identifiers.parse_swhid: Make SWHIDs with whitespaces invalid.Nov 12 2020, 10:42 AM
ardumont updated this revision to Diff 15824.Nov 12 2020, 11:04 AM
  • Rebase
  • Adapt according to previous diff's review
swh-public-ci added a comment.Nov 12 2020, 11:05 AM

Build is green

Patch application report for D4461 (id=15824)

Could not rebase; Attempt merge onto 4e3fdc062c...

Updating 4e3fdc0..20d7361
Fast-forward
 swh/model/identifiers.py            | 35 ++++++++++++-----
 swh/model/tests/test_identifiers.py | 77 ++++++++++++++++++++++---------------
 2 files changed, 72 insertions(+), 40 deletions(-)
Changes applied before test
commit 20d736195e8ec28918b90ec598a672fabedee702
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Nov 12 10:08:45 2020 +0100

    identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid
    
    Related to T2769

commit 8130a27e0261962bc5b36bddf914856ca24d15d2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 18:21:01 2020 +0100

    model.identifiers: Improve error messages in case of invalid SWHIDs
    
    Related to T2769

commit 47946aac0220fcfd55c7998908c71a6f402a5ddb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 16:11:32 2020 +0100

    test: Migrate parse_swhid test cases to pytest
    
    Related to T2769

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

Harbormaster completed remote builds in B17034: Diff 15824.Nov 12 2020, 11:05 AM
vlorentz added a subscriber: vlorentz.Nov 12 2020, 11:14 AM
vlorentz added inline comments.
swh/model/identifiers.py
849–866

You can remove the CONTEXT_QUALIFIERS check from the loop, then test after the loop if set(_metadata) - set(CONTEXT_QUALIFIERS) is empty. Then, you can show which qualifiers are invalid (helpful if it's a typo and there are many qualifiers).

swh/model/tests/test_identifiers.py
1061–1070

what does "off" mean?

and you can move # noqa at the end of the list so you don't have to repeat it, iirc

ardumont updated this revision to Diff 15827.Nov 12 2020, 11:14 AM

Rebase

swh-public-ci added a comment.Nov 12 2020, 11:17 AM

Build is green

Patch application report for D4461 (id=15827)

Could not rebase; Attempt merge onto 4e3fdc062c...

Updating 4e3fdc0..1a71edf
Fast-forward
 swh/model/identifiers.py            | 35 ++++++++++++-----
 swh/model/tests/test_identifiers.py | 77 ++++++++++++++++++++++---------------
 2 files changed, 72 insertions(+), 40 deletions(-)
Changes applied before test
commit 1a71edf9830257f300605bebaf097b89b52e4d8a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Nov 12 10:08:45 2020 +0100

    identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid
    
    Related to T2769

commit 22c7c888870986e9303554184f42d4429d4b2ba2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 18:21:01 2020 +0100

    model.identifiers: Improve error messages in case of invalid SWHIDs
    
    Related to T2769

commit 47946aac0220fcfd55c7998908c71a6f402a5ddb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 16:11:32 2020 +0100

    test: Migrate parse_swhid test cases to pytest
    
    Related to T2769

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

Harbormaster completed remote builds in B17037: Diff 15827.Nov 12 2020, 11:17 AM
ardumont added inline comments.Nov 12 2020, 11:20 AM
swh/model/tests/test_identifiers.py
1061–1070

off as in off course so in the end, i mean`wrong` but it's 3 characters more to typ ¯\_(ツ)_/¯

i'll check the noqa hint (i'm not sure it works)

thanks.

ardumont updated this revision to Diff 15829.Nov 12 2020, 11:29 AM

Adapt according to review

swh/model/tests/test_identifiers.py
1061–1070

nah, it complains [1], i gather what you had in mind was for the docstring.

[1]

$ flake8 swh
swh/model/tests/test_identifiers.py:1063:89: E501 line too long (99 > 88 characters)
swh/model/tests/test_identifiers.py:1065:89: E501 line too long (99 > 88 characters)
swh/model/tests/test_identifiers.py:1067:89: E501 line too long (100 > 88 characters)
swh/model/tests/test_identifiers.py:1069:89: E501 line too long (100 > 88 characters)
swh/model/tests/test_identifiers.py:1071:89: E501 line too long (110 > 88 characters)
swh/model/tests/test_identifiers.py:1074:89: E501 line too long (101 > 88 characters)
swh/model/tests/test_identifiers.py:1075:89: E501 line too long (109 > 88 characters)
ardumont marked 2 inline comments as done.Nov 12 2020, 11:29 AM
ardumont added inline comments.
swh/model/identifiers.py
849–866

thanks!

swh/model/tests/test_identifiers.py
1061–1070

renamed off to wrong.

swh-public-ci added a comment.Nov 12 2020, 11:30 AM

Build is green

Patch application report for D4461 (id=15829)

Could not rebase; Attempt merge onto 4e3fdc062c...

Updating 4e3fdc0..7136504
Fast-forward
 swh/model/identifiers.py            | 38 +++++++++++++-----
 swh/model/tests/test_identifiers.py | 77 ++++++++++++++++++++++---------------
 2 files changed, 75 insertions(+), 40 deletions(-)
Changes applied before test
commit 71365044b5f386da3827ccc29184d640256fb070
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Nov 12 10:08:45 2020 +0100

    identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid
    
    Related to T2769

commit 22c7c888870986e9303554184f42d4429d4b2ba2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 18:21:01 2020 +0100

    model.identifiers: Improve error messages in case of invalid SWHIDs
    
    Related to T2769

commit 47946aac0220fcfd55c7998908c71a6f402a5ddb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Nov 10 16:11:32 2020 +0100

    test: Migrate parse_swhid test cases to pytest
    
    Related to T2769

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

Harbormaster completed remote builds in B17039: Diff 15829.Nov 12 2020, 11:30 AM
ardumont mentioned this in T2769: Make function swh.model.identifiers.parse_swhid more strict.Nov 12 2020, 11:53 AM
vlorentz added inline comments.Nov 12 2020, 11:58 AM
swh/model/identifiers.py
862

s/" The qualifier must be one of/"The qualifiers must be/

(space + plural)

ardumont updated this revision to Diff 15836.Nov 12 2020, 12:38 PM
ardumont marked an inline comment as done.

Fix space

(the singular was voluntary but ok, it's clearer since i've used the
plural on the first sentence anyway)

swh-public-ci added a comment.Nov 12 2020, 12:39 PM

Build is green

Patch application report for D4461 (id=15836)

Rebasing onto 22c7c88887...

Current branch diff-target is up to date.
Changes applied before test
commit fb504b400ba90fe8a0815769b6a431489fc8a560
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Nov 12 10:08:45 2020 +0100

    identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid
    
    Related to T2769

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

Harbormaster completed remote builds in B17046: Diff 15836.Nov 12 2020, 12:39 PM
vlorentz accepted this revision.Nov 12 2020, 12:56 PM
This revision is now accepted and ready to land.Nov 12 2020, 12:56 PM
Closed by commit rDMODfb504b400ba9: identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid (authored by ardumont). · Explain WhyNov 12 2020, 12:58 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDMODfb504b400ba9: identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid.

Revision Contents
Changeset List

PathSize
swh/
model/
16 lines
tests/
10 lines

Diff 15838

View Options

swh/model/identifiers.py

Loading...
View Options

swh/model/tests/test_identifiers.py

Loading...
About · Developer information · Legal