Differential D1077

Add more type checks in metadata dictionary.
ClosedPublic
Actions

Authored by vlorentz on Feb 5 2019, 10:16 AM.

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Commits

rDCIDXa0f6fe3ea02d: Fix bugs found by hypothesis.
rDCIDX0f75ae1e7346: Use hypothesis to generate adversarial inputs.
rDCIDXd3a6ae1930ff: Add more type checks in metadata dictionary.

Diff Detail

Repository

rDCIDX Metadata indexer

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.Feb 5 2019, 10:16 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 5 2019, 10:16 AM

Build is green
See https://jenkins.softwareheritage.org/job/DCIDX/job/tox/313/ for more details.

Harbormaster completed remote builds in B4010: Diff 3390.Feb 5 2019, 10:19 AM

As always with this kind of diff, I'm quite puzzled: we add a lot of 'if' conditions (on types), but what are the unexpected types that the code may encounter? As is, we just (silently) ignore a bunch of values.

I get it that we parse (normally) structured files from the wide, so we shall expect nothing valid. But shouldn't we at least notify the failure? shouldn't there be a metedata entry that state the parsing was successful, partial or a complete failure?

Also, you add many defensive statements, but none of them are actually tested, unless I'm mistaken. Once again, please add tests for these invalid input resiliency checks.

douardda requested changes to this revision.Feb 7 2019, 9:33 AM

This revision now requires changes to proceed.Feb 7 2019, 9:33 AM

Use hypothesis to generate adversarial inputs.
Fix bugs found by hypothesis.

Harbormaster failed remote builds in B4120: Diff 3491!Feb 8 2019, 3:29 PM

Build has FAILED

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

Build has FAILED

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

rebase

Build has FAILED

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

Harbormaster failed remote builds in B4122: Diff 3492!Feb 8 2019, 3:42 PM

Fix doctest typo
Smaller hypothesis examples.

Build is green
See https://jenkins.softwareheritage.org/job/DCIDX/job/tox/384/ for more details.

Harbormaster completed remote builds in B4123: Diff 3493.Feb 8 2019, 3:47 PM

Fix another bug found by hypothesis.

Build has FAILED

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

Harbormaster failed remote builds in B4124: Diff 3494!Feb 8 2019, 4:23 PM

Build is green
See https://jenkins.softwareheritage.org/job/DCIDX/job/tox/387/ for more details.

Let's keep on, but I really don't like the turn this code takes...

This revision is now accepted and ready to land.Feb 12 2019, 9:32 AM

Rebase + squash

Closed by commit rDCIDXd3a6ae1930ff: Add more type checks in metadata dictionary. (authored by vlorentz). · Explain WhyFeb 12 2019, 12:20 PM

This revision was automatically updated to reflect the committed changes.

Harbormaster failed remote builds in B4158: Diff 3528!Feb 12 2019, 12:20 PM

Build has FAILED

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

Revision Contents
Changeset List

Path

Size

swh/

indexer/

metadata_dictionary.py

19 lines

tests/

test_metadata.py

12 lines

Diff 3529

swh/indexer/metadata_dictionary.py

Loading...

swh/indexer/tests/test_metadata.py

Loading...