Paths

Table of Contentst

Differential D6341

model: Replace attrs-strict with stricter validation
ClosedPublic
Actions

Authored by vlorentz on Sep 24 2021, 2:26 PM.

Details

Reviewers

douardda

Group Reviewers

Reviewers

Commits

rDMOD734b0812c165: model: Replace attrs-strict with stricter validation

Summary

This reimplements attrs_strict.type_validator(), using type equality
instead of isinstance.

This makes my checksum validation script (that mostly just instantiates
model objects, computes a checksum, then discard) run twice as fast.

I'm reusing the exception from attrs_strict for compatibility with
existing code that might rely on it. (Though I don't think there's any).

If you're fine with this diff in principle, I'll add tests for this
new type checker.

Diff Detail

Repository

rDMOD Data model

Branch

attrs-stricter

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 23983
Build 37415: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 37414: arc lint + arc unit

Event Timeline

vlorentz created this revision.Sep 24 2021, 2:26 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptSep 24 2021, 2:26 PM

Build is green

Patch application report for D6341 (id=23056)

Rebasing onto e30eb7d291...

Current branch diff-target is up to date.

Changes applied before test

commit 9318d3b15096e349c43bf0650e4641eae0a0e06e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Sep 24 14:24:36 2021 +0200

    model: Replace attrs-strict with stricter validation
    
    This reimplements attrs_strict.type_validator(), using type equality
    instead of isinstance.
    
    This makes my checksum validation script (that mostly just instantiates
    model objects, computes a checksum, then discard) run twice as fast.

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

Harbormaster completed remote builds in B23983: Diff 23056.Sep 24 2021, 2:28 PM

vlorentz requested review of this revision.Sep 24 2021, 2:28 PM

olasd added a subscriber: olasd.Sep 24 2021, 5:38 PM

olasd added inline comments.

swh/model/model.py
137

Looks fine to me, but it needs some extensive tests indeed.

swh/model/model.py
96	we kind of need it Out of curiosity, why?
102	Why `origin not in (Union,)` instead of `origin is not Union` ?
122	Might be useful to ad a comment on why there is nothing related to standard dict, maybe.

vlorentz added inline comments.Sep 27 2021, 10:28 AM

swh/model/model.py
96	directory entry permissions. I guess we could add a converter from DentryPerms to int though.
102	I expected I would need more when writing it, but it turns out I don't

add extensive testing
remove special-casing of IntEnum and add converter for Directory.perms
replace origin not in (Union,)
fix a typo (raise False)
add comment explaining why we don't check for dict and list

Build is green

Patch application report for D6341 (id=23119)

Rebasing onto e30eb7d291...

Current branch diff-target is up to date.

Changes applied before test

commit 734b0812c165892039b2a9bc74e51a29e21fb0f9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Sep 24 14:24:36 2021 +0200

    model: Replace attrs-strict with stricter validation
    
    This reimplements attrs_strict.type_validator(), using type equality
    instead of isinstance.
    
    This makes my checksum validation script (that mostly just instantiates
    model objects, computes a checksum, then discard) run twice as fast.

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

Harbormaster completed remote builds in B24036: Diff 23119.Sep 28 2021, 11:55 AM

Test coverage looks fairly complete, thx

This revision is now accepted and ready to land.Sep 29 2021, 10:45 AM

Closed by commit rDMOD734b0812c165: model: Replace attrs-strict with stricter validation (authored by vlorentz). · Explain WhySep 29 2021, 3:52 PM

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDMOD734b0812c165: model: Replace attrs-strict with stricter validation.