Use Tuple instead of List in model declarations

Use Tuple instead of List in model declaration.

---

Migrated from D3177 (view on Phabricator)

Build has FAILED

Patch application report for D3177 (id=11284)

Rebasing onto 29312dff...

Current branch diff-target is up to date.

Changes applied before test

commit e88599c843d123e7689c39a072600c48c3050d80
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon May 25 13:17:00 2020 +0200

    Make all model objects hashable
    
    Use Tuple instead of List in model declaration.
    
    This requires currently not yet published attrs_strict PR #38.

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/64/ See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/64/console

Bump dep on attrs_strict >= 0.0.7

now it's on pypi.

Build has FAILED

Patch application report for D3177 (id=11285)

Rebasing onto 29312dff...

Current branch diff-target is up to date.

Changes applied before test

commit 50ba1c389f0b64bfacb07de4bb1152bc87bb67d8
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon May 25 13:17:00 2020 +0200

    Make all model objects hashable
    
    Use Tuple instead of List in model declaration.
    
    This requires attrs_strict >= 0.0.7.

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/65/ See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/65/console

Forgot to update hypothesis strategies also.

Build is green

Patch application report for D3177 (id=11286)

Rebasing onto 29312dff...

Current branch diff-target is up to date.

Changes applied before test

commit 9cb641e3d52ba468f8a28fde05507752764da6c9
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon May 25 13:17:00 2020 +0200

    Make all model objects hashable
    
    Use Tuple instead of List in model declaration.
    
    This requires attrs_strict >= 0.0.7.

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

What is the motivation for this?

! In !113 (closed), @vlorentz wrote: What is the motivation for this?

Be able to create sets of revisions.

! In !113 (closed), @douardda wrote:

! In !113 (closed), @vlorentz wrote: What is the motivation for this?

Be able to create sets of revisions.

Also, having frozen objects that are modifiable seems a bit fuzzy to me, to say the least.

! In !113 (closed), @douardda wrote:

! In !113 (closed), @douardda wrote:

! In !113 (closed), @vlorentz wrote: What is the motivation for this?

Be able to create sets of revisions.

Also, having frozen objects that are modifiable seems a bit fuzzy to me, to say the least.

But I agree I should have explained the "why", and not (only) the "what", in the commit message.

improve the commit message

Build is green

Patch application report for D3177 (id=11287)

Rebasing onto 29312dff...

Current branch diff-target is up to date.

Changes applied before test

commit ffb0ba3f719517bed7ae43464ee403ec36598970
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon May 25 13:17:00 2020 +0200

    Make all model objects actually frozen and thus hashable
    
    Use Tuple instead of List in model declarations. This is needed so all model
    objects, declared as frozen, are really no editable.
    
    This also allows to create sets of model objects (especially for Revision)
    which can be handy for tests (eg. when testing a replayer scenario, checking
    all objects of a type has been replayed using simple comparisons of sets.)
    
    This requires attrs_strict >= 0.0.7.

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

We wouldn't need that if pytest had an equivalent for assertCountEqual :/

And this diff is missing tests to check the objects are indeed hashable

Merge request was returned for changes

! In !113 (closed), @vlorentz wrote: We wouldn't need that if pytest had an equivalent for assertCountEqual :/

That's only a side effect, even if it's the original reason I started this. Having a mutable "frozen" object is just bad design IMHO.

And this diff is missing tests to check the objects are indeed hashable

Yes, I've added one, which fails due to dicts...

! In !113 (closed), @douardda wrote:

! In !113 (closed), @vlorentz wrote: And this diff is missing tests to check the objects are indeed hashable

Yes, I've added one, which fails due to dicts...

We need to move the extra_headers away from Revision.metadata into a top-level, immutable object, as this is a significant field that is used in the revision id.

Once that's done, we can ignore the fact that we still have mutable metadata in these objects, considering that the field is on the way out, superseded by the extrinsic metadata storage which should be happening real soon.

The commit message should at least be amended, since it does not make all model objects immutable, but only fix a couple of model classes.

Bump dep on attrs_strict >= 0.0.7 now it's on pypi.

Now it's on our debian repository as well :D I have taken the opportunity to also update our wiki page on debian packaging \m/ [1]

• [1] https://wiki.softwareheritage.org/wiki/Debian_packaging#Updating_a_dependency_packaging_repository

rebase + rephrase the commit message

Build is green

Patch application report for D3177 (id=11377)

Rebasing onto a95646fc...

Current branch diff-target is up to date.

Changes applied before test

commit bc2c250b60db9b8562c60cafcb2fdda7ac6cc6b8
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon May 25 13:17:00 2020 +0200

    Use Tuple instead of List in model declarations.
    
    This is a step forward having model objects, declared as frozen, immutable.
    
    This requires attrs_strict >= 0.0.7.

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

Looks fine to me (and in accord with the associated task discussion ;)