Differential D2470

Improve validation of --author and --name.
ClosedPublic
Actions

Authored by vlorentz on Dec 18 2019, 6:54 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Commits

rDDEPe25693b4901b: Improve validation of --author and --name.

Summary

Mostly, the code used to allow having either --author or --name,
while it should use either both (when generating metadata) or
neither (when using a metadata file).

This code fixes this, and also catches some more invalid cases.

Depends on D2469.

Diff Detail

Repository

rDDEP Push deposit

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.Dec 18 2019, 6:54 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptDec 18 2019, 6:54 PM

Harbormaster failed remote builds in B9868: Diff 8771!Dec 18 2019, 6:55 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DDEP/job/tox/216/
See console output for more information: https://jenkins.softwareheritage.org/job/DDEP/job/tox/216/console

remove file from commit

Harbormaster failed remote builds in B9869: Diff 8772!Dec 18 2019, 6:56 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DDEP/job/tox/217/
See console output for more information: https://jenkins.softwareheritage.org/job/DDEP/job/tox/217/console

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DDEP/job/tox/221/
See console output for more information: https://jenkins.softwareheritage.org/job/DDEP/job/tox/221/console

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DDEP/job/tox/224/
See console output for more information: https://jenkins.softwareheritage.org/job/DDEP/job/tox/224/console

looks about right

wondering if we are not missing some valid case to demo

swh/deposit/tests/cli/test_client.py
207	Don't we want to also show that the correct combinations work: --author and --name --metadata whatever i'm missing else cases ;)

ardumont added inline comments.Dec 19 2019, 9:20 AM

swh/deposit/cli/client.py
206	We could be lenient and drop the unneeded information. By introspecting both metadata and flags then trying to determine what could be dropped. In case of too much corner case and thus conflict, then raising and error to ask the clients what they really want ;) That's a lot more work though...

build failure

I think that misses some rebase

ardumont accepted this revision.Dec 19 2019, 9:55 AM

This revision is now accepted and ready to land.Dec 19 2019, 9:55 AM

vlorentz added inline comments.Dec 19 2019, 1:36 PM

swh/deposit/cli/client.py
206	By introspecting both metadata and flags then trying to determine what could be dropped. That's already what I'm doing. But if the user is giving contradictory arguments, you can't tell which way they should correct it.
swh/deposit/tests/cli/test_client.py
207	--author and --name Tested in test_single_minimal_deposit --metadata Tested in test_multisteps_deposit.