Paths

Table of Contentst

Differential D4936

deposit.cli: Warn users when missing origin tags are detected
ClosedPublic
Actions

Authored by ardumont on Jan 25 2021, 12:33 PM.

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Maniphest Tasks

T2860: Change parameter `slug` in swh-deposit cli to `create_origin`

Commits

rDDEP4deee5ba948b: deposit.cli: Warn users when missing origin tags are detected

Summary

This explicitely checks for the presence of swh:origin_to_add or swh:create_origin
within the provided metadata file.

If not present, this logs a warning about it.

Related to T2860

Diff Detail

Repository

rDDEP Push deposit

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 18684
Build 28917: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 28916: arc lint + arc unit

Event Timeline

ardumont created this revision.Jan 25 2021, 12:33 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJan 25 2021, 12:33 PM

Harbormaster completed remote builds in B18684: Diff 17551.Jan 25 2021, 12:36 PM

Build is green

Patch application report for D4936 (id=17551)

Rebasing onto b65694973e...

Current branch diff-target is up to date.

Changes applied before test

commit 3eb7840a32eec961d072b2a8807a08bd0c110855
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Jan 25 12:32:15 2021 +0100

    deposit.cli: Warn users when missing origin tags are detected
    
    This explicitely checks for the presence of swh:origin_to_add or swh:create_origin
    within the provided metadata file.
    
    If not present, this logs a warning about it.
    
    Related to T2860

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

ardumont requested review of this revision.Jan 25 2021, 12:36 PM

vlorentz requested changes to this revision.Jan 25 2021, 3:44 PM

vlorentz added a subscriber: vlorentz.

vlorentz added inline comments.

swh/deposit/cli/client.py
263–266
swh/deposit/tests/cli/test_client.py
810
842–877	merge this with the regular tests; and check there is no log with level >= WARNING at all, that's an even better test.

This revision now requires changes to proceed.Jan 25 2021, 3:44 PM

Thanks for the suggestions, will do.

ardumont added inline comments.Jan 25 2021, 4:01 PM

swh/deposit/tests/cli/test_client.py
842–877	it's the other way around: 40 -> error, 30 -> warning, 20 -> info, ... but ok :)

Adapt according to review

Build is green

Patch application report for D4936 (id=17587)

Rebasing onto b65694973e...

Current branch diff-target is up to date.

Changes applied before test

commit c841f8b473594f6748d933dfe57bb815ac3a8bf1
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Jan 25 12:32:15 2021 +0100

    deposit.cli: Warn users when missing origin tags are detected
    
    This explicitely checks for the presence of swh:origin_to_add or swh:create_origin
    within the provided metadata file.
    
    If not present, this logs a warning about it.
    
    Related to T2860

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

Harbormaster completed remote builds in B18725: Diff 17587.Jan 25 2021, 4:06 PM

ardumont marked 3 inline comments as done.Jan 25 2021, 4:51 PM

sorry, I didn't mean merging the non-error test with the error test, but to merge the non-error assertion with other tests (ie. the one that already don't trigger errors)

sorry, I didn't mean merging the non-error test with the error test, but to merge the non-error assertion with other tests (ie. the one that already don't trigger errors)

the funny thing is that i understood it that way initially...
got distracted..,
came back here....
and made a mess ¯\_(ツ)_/¯

sorry, I didn't mean merging the non-error test with the error test, but to merge the non-error assertion with other tests (ie. the one that already don't trigger errors)

the funny thing is that i understood it that way initially...
got distracted..,
came back here....
and made a mess ¯\_(ツ)_/¯

What you suggest sounds good but it's hellishly annoying to do in the current state...
so not right now...

ardumont planned changes to this revision.Jan 26 2021, 9:54 AM

ardumont added inline comments.Jan 27 2021, 5:45 PM

swh/deposit/cli/client.py
131	Found a bug! it's missing the ['swh:deposit'] level.

Adapt according to review

Only adapted 2 nominal scenarios with it.

I kept the last failing scenario as it also tests the add_to_origin (and we had none
about it yet)

This caught a bug in the metadata generation about the <create_origin> case. It was
missing the swh:deposit level.

Build is green

Patch application report for D4936 (id=17680)

Rebasing onto b65694973e...

Current branch diff-target is up to date.

Changes applied before test

commit 4deee5ba948ba6cb6760772b235371eace4c9690
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Jan 25 12:32:15 2021 +0100

    deposit.cli: Warn users when missing origin tags are detected
    
    This explicitely checks for the presence of swh:origin_to_add or swh:create_origin
    within the provided metadata file.
    
    If not present, this logs a warning about it.
    
    Related to T2860

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

Harbormaster completed remote builds in B18821: Diff 17680.Jan 27 2021, 5:51 PM

vlorentz accepted this revision.Feb 1 2021, 3:51 PM

This revision is now accepted and ready to land.Feb 1 2021, 3:51 PM

Closed by commit rDDEP4deee5ba948b: deposit.cli: Warn users when missing origin tags are detected (authored by ardumont). · Explain WhyFeb 1 2021, 4:55 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDDEP4deee5ba948b: deposit.cli: Warn users when missing origin tags are detected.

ardumont mentioned this in T2860: Change parameter `slug` in swh-deposit cli to `create_origin`.Feb 1 2021, 4:56 PM