Page MenuHomeSoftware Heritage

model: use attrs_static to enforce type validation of model objects
ClosedPublic

Authored by douardda on Mar 11 2020, 5:56 PM.

Details

Summary

This does add runtime validation, and provides type validation by
default on swh.model entities.

Related to T2308.

Depends on D2818.

Diff Detail

Event Timeline

douardda created this revision.Mar 11 2020, 5:56 PM
olasd requested changes to this revision.Mar 11 2020, 8:01 PM
olasd added a subscriber: olasd.

I don't hate the idea (although we're missing a Debian package for this). I've suggested a few changes.

I don't understand the "runtime validation" comment in the diff description. AFAIK validators from attrs _are_ indeed run when the objects are instantiated (and the documentation seems to back me up).

mypy.ini
4

looks like something that doesn't exist (yet?)

swh/model/model.py
38

Can we get a proper name for this function? something like attrib_typecheck would be way more sensible.

It'd also be nice to support extra validators to be passed as arguments (though I guess our current attributes with validators are already stricter than a plain typecheck). If we do that then we can have all our attributes use this consistently, I guess.

This revision now requires changes to proceed.Mar 11 2020, 8:01 PM
ardumont added inline comments.
requirements.txt
6

Is it in debian?

swh/model/model.py
600–601

can't you adapt ibv to allow the validator override?

douardda updated this revision to Diff 10017.Mar 12 2020, 2:30 PM

rebase + add missing plugin file

douardda updated this revision to Diff 10027.Mar 12 2020, 4:07 PM

add support for the 'validator' argument in attrib_typecheck

douardda updated this revision to Diff 10030.Mar 12 2020, 4:22 PM

add missing mypy deps in requirements-test

olasd accepted this revision.Mar 16 2020, 2:17 PM
This revision is now accepted and ready to land.Mar 16 2020, 2:17 PM
douardda updated this revision to Diff 10271.Mar 26 2020, 11:29 AM

get rid of the attrib_typecheck() decorator to prevent the need for a mypy plugin

which would then have required to modify the mypy.ini file in every other swh
package.

douardda edited the summary of this revision. (Show Details)Mar 27 2020, 11:06 AM

Build is green

Patch application report for D2819 (id=10440)

Rebasing onto e9a4c7519e...

Current branch diff-target is up to date.
Changes applied before test
commit 85ca7d7848008951f2e26c55c1c72ed9fa92cefb
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Mar 20 12:59:56 2020 +0100

    model: use attrs_static to enforce type validation of model objects
    
    This ensures all instanciated model entities have valid types for attributes.
    
    Related to T2308.

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