Page MenuHomeSoftware Heritage

Add an `object_type` attribute to model classes
Closed, ResolvedPublic

Description

With the current move to using swh.model objects as much as possible, we no longer need to also pass the object type as argument. See eg. the JournalWriter.write_additions() for example:

def write_additions(self, obj_type, values) -> None:

in which values is expected to be an iterable of model objects of object_type type.

In order to prevent the usage of is_instance everywhere, a simple solution is to add an attribute to swh.model.model classes (typically `.object_type') as Final attributes.

This idea has been proposed in D3152, however, the current implementation is stuck because of the way lazy-loading capable Content are implemented (subclassing the Content model class).

Event Timeline

douardda triaged this task as Normal priority.May 26 2020, 5:03 PM
douardda created this task.
douardda added a comment.EditedWed, Jun 24, 3:28 PM

This task is currently blocked by an implementation "detail":

  • the swh.model.model.Content class is subclassed by the swh.model.from_dict.DiskBackedContent to provide a lazy-loading mechanism for Content objects which data is actually a disk file.
  • the proposed diff (D3152) make the assumption a model class with the object_type attribute should not be subclassed to prevent having to deal with a poor-man's parallel type system (so the test test_object_type_is_final provided in this diff, however this later currently fails because of the presence of swh.model.from_dict.DiskBackedContent.

A short-term solution is to release this must-be-final property and add an object_type attribute to from_dict.DiskBackedContent with a value like "content-file" or something.

But a closer look at the code shows that this lazyness property of from_dict.Content is currently unused (used to be, but refactoring got rid of it).

So the lazyness property of a model.Content should be rethought.

I propose to do what is described above to make D3152 land, and create a new ticket dedicated to how to properly reimplement this lazy-loading behavior.

Previously proposed "short-term" solution does not work. So the only "short-term" solution is to make DiskBakedContent inherit from BaseModel (or BaseContent).