Page MenuHomeSoftware Heritage

storage/writer: refactor JournalWriter.content_add to send model objects
ClosedPublic

Authored by douardda on Tue, Mar 10, 4:46 PM.

Details

Summary

to the journal writer, as it already does with other objet types
(instead of dicts).

Diff Detail

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

douardda created this revision.Tue, Mar 10, 4:46 PM
ardumont accepted this revision.Tue, Mar 10, 5:03 PM
This revision is now accepted and ready to land.Tue, Mar 10, 5:03 PM
olasd added a subscriber: olasd.Tue, Mar 10, 5:06 PM

What are the implications of this on the JournalClient side?

ardumont added a comment.EditedTue, Mar 10, 5:08 PM

What are the implications of this on the JournalClient side?

I think it's fine.
write_additions knows how to deal with BaseModel objects.
It transforms it into dict for now.

if isinstance(object_, BaseModel):
    object_ = object_.to_dict()

[1] https://forge.softwareheritage.org/source/swh-journal/browse/master/swh/journal/writer/kafka.py$93-96

olasd added a comment.Tue, Mar 10, 5:22 PM

What are the implications of this on the JournalClient side?

I think it's fine.
write_additions knows how to deal with BaseModel objects.
It transforms it into dict for now.

if isinstance(object_, BaseModel):
    object_ = object_.to_dict()

[1] https://forge.softwareheritage.org/source/swh-journal/browse/master/swh/journal/writer/kafka.py$93-96

So the only change on the wire will be that dicts representing contents will have a new 'data': None item. Fine.

My main doubt was whether we stopped explicitly converting model objects to dicts altogether (going through the swh.core model serializer instead). But even in that case contents will still be deserializable (as Content.from_dict(d) still works even when d['data'] is None).

In D2803#67024, @olasd wrote:

My main doubt was whether we stopped explicitly converting model objects to dicts altogether (going through the swh.core model serializer instead). But even in that case contents will still be deserializable (as Content.from_dict(d) still works even when d['data'] is None).

What swh.core model serializer do you refer to? The ones in swh.core.api?

In D2803#67024, @olasd wrote:

My main doubt was whether we stopped explicitly converting model objects to dicts altogether (going through the swh.core model serializer instead). But even in that case contents will still be deserializable (as Content.from_dict(d) still works even when d['data'] is None).

What swh.core model serializer do you refer to? The ones in swh.core.api?

Yes. And now that you've pointed it out, I've remembered that it's the swh.storage RPC layer that adds a hook to support model objects.

In D2803#67209, @olasd wrote:
In D2803#67024, @olasd wrote:

My main doubt was whether we stopped explicitly converting model objects to dicts altogether (going through the swh.core model serializer instead). But even in that case contents will still be deserializable (as Content.from_dict(d) still works even when d['data'] is None).

What swh.core model serializer do you refer to? The ones in swh.core.api?

Yes. And now that you've pointed it out, I've remembered that it's the swh.storage RPC layer that adds a hook to support model objects.

Ah ok, makes more sense then ;-)