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

Repository
rDMOD Data model
Branch
typecheck
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 11350
Build 17177: tox-on-jenkinsJenkins
Build 17176: arc lint + arc unit

Event Timeline

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
37

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
5

Is it in debian?

swh/model/model.py
573–574

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

rebase + add missing plugin file

add support for the 'validator' argument in attrib_typecheck

add missing mypy deps in requirements-test

This revision is now accepted and ready to land.Mar 16 2020, 2:17 PM

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.

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.