Page MenuHomeSoftware Heritage

Add more type checks in metadata dictionary.
ClosedPublic

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

Diff Detail

Repository
rDCIDX Object 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.Tue, Feb 5, 10:16 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.Thu, Feb 7, 9:33 AM
This revision now requires changes to proceed.Thu, Feb 7, 9:33 AM
vlorentz updated this revision to Diff 3491.Fri, Feb 8, 3:28 PM
  • Use hypothesis to generate adversarial inputs.
  • Fix bugs found by hypothesis.
vlorentz updated this revision to Diff 3492.Fri, Feb 8, 3:40 PM
  • rebase
vlorentz updated this revision to Diff 3493.Fri, Feb 8, 3:44 PM
  • Fix doctest typo
  • Smaller hypothesis examples.
vlorentz updated this revision to Diff 3494.Fri, Feb 8, 4:20 PM
  • Fix another bug found by hypothesis.
douardda accepted this revision.Tue, Feb 12, 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.Tue, Feb 12, 9:32 AM
vlorentz updated this revision to Diff 3528.Tue, Feb 12, 12:19 PM
  • Rebase + squash
This revision was automatically updated to reflect the committed changes.