⚙ D4461 identifiers.parse_swhid: Check the swhid qualifiers and fail if invalid

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.

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

Fix noisy characters in noqa comments
add comments to explicit what's wrong with swhid

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

Rebase

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

Rebase
Adapt according to previous diff's review

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–863	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
1062–1071	what does "off" mean? and you can move `# noqa` at the end of the list so you don't have to repeat it, iirc

Rebase

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
1062–1071	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.

Adapt according to review

swh/model/tests/test_identifiers.py

1062–1071

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–863	thanks!
swh/model/tests/test_identifiers.py
1062–1071	renamed off to wrong.

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
868	`s/" The qualifier must be one of/"The qualifiers must be/` (space + plural)

Fix space

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

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.

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

Details

Diff Detail

Event Timeline

Patch application report for D4461 (id=15816)

Changes applied before test

Patch application report for D4461 (id=15819)

Changes applied before test

Patch application report for D4461 (id=15821)

Changes applied before test

Patch application report for D4461 (id=15824)

Changes applied before test

Patch application report for D4461 (id=15827)

Changes applied before test

Patch application report for D4461 (id=15829)

Changes applied before test

Patch application report for D4461 (id=15836)

Changes applied before test

Revision Contents
Changeset List

Diff 15824

swh/model/identifiers.py

swh/model/tests/test_identifiers.py

identifiers.parse_swhid: Check the swhid qualifiers and fail if invalidClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D4461 (id=15816)

Changes applied before test

Patch application report for D4461 (id=15819)

Changes applied before test

Patch application report for D4461 (id=15821)

Changes applied before test

Patch application report for D4461 (id=15824)

Changes applied before test

Patch application report for D4461 (id=15827)

Changes applied before test

Patch application report for D4461 (id=15829)

Changes applied before test

Patch application report for D4461 (id=15836)

Changes applied before test

Revision ContentsChangeset List

Diff 15824

swh/model/identifiers.py

swh/model/tests/test_identifiers.py

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

Revision Contents
Changeset List