Page MenuHomeSoftware Heritage
Differential D1077

Add more type checks in metadata dictionary.
ClosedPublic
Actions

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

Details

Reviewers
douardda
Group Reviewers
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
Branch
more-type-checks
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 4010
Build 5258: tox-on-jenkinsJenkins
Build 5257: arc lint + arc unit

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
swh-public-ci added a subscriber: swh-public-ci.Feb 5 2019, 10:19 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
douardda added a subscriber: douardda.Feb 7 2019, 9:30 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?

douardda added a comment.Feb 7 2019, 9:32 AM

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
vlorentz updated this revision to Diff 3491.Feb 8 2019, 3:28 PM
  • 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
swh-public-ci added a comment.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

swh-public-ci added a comment.Feb 8 2019, 3:33 PM

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

vlorentz updated this revision to Diff 3492.Feb 8 2019, 3:40 PM
  • rebase
swh-public-ci added a comment.Feb 8 2019, 3:42 PM

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
vlorentz updated this revision to Diff 3493.Feb 8 2019, 3:44 PM
  • Fix doctest typo
  • Smaller hypothesis examples.
swh-public-ci added a comment.Feb 8 2019, 3:46 PM

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
vlorentz updated this revision to Diff 3494.Feb 8 2019, 4:20 PM
  • Fix another bug found by hypothesis.
swh-public-ci added a comment.Feb 8 2019, 4:23 PM

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
swh-public-ci added a comment.Feb 8 2019, 4:58 PM

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

douardda accepted this revision.Feb 12 2019, 9:32 AM

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
vlorentz updated this revision to Diff 3528.Feb 12 2019, 12:19 PM
  • 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
swh-public-ci added a comment.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

PathSize
swh/
indexer/
19 lines
tests/
12 lines

Diff 3390

View Options

swh/indexer/metadata_dictionary.py

Loading...
View Options

swh/indexer/tests/test_metadata.py

Loading...
About · Developer information · Legal