Move _get_key from the kafka writer to serializers
ClosedPublic
Actions

Authored by olasd on Apr 10 2020, 1:06 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Commits

rDJNLcc5797df4b08: Move _get_key from the kafka writer to serializers

Summary

Use the opportunity to add some trivial smoke tests

Depends on D3004.

Test Plan

tox + new tests

Diff Detail

Repository

rDJNL Journal infrastructure

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

olasd created this revision.Apr 10 2020, 1:06 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptApr 10 2020, 1:06 PM

Build is green

Patch application report for D3005 (id=10667)

Could not rebase; Attempt merge onto 750d4c6c5b...

Updating 750d4c6..fb21467
Fast-forward
 swh/journal/serializers.py             | 64 +++++++++++++++++++++++++++++++++-
 swh/journal/tests/conftest.py          | 29 ++++++++++++---
 swh/journal/tests/test_kafka_writer.py | 55 +++++++++++------------------
 swh/journal/tests/test_replay.py       | 42 ++++++++++++----------
 swh/journal/tests/test_serializers.py  |  9 +++++
 swh/journal/writer/kafka.py            | 60 ++++++-------------------------
 6 files changed, 151 insertions(+), 108 deletions(-)

Changes applied before test

commit fb21467e8864d2e6e71601038ef0a160b1a33a4f
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 9 17:58:40 2020 +0200

    Move _get_key from the kafka writer to serializers
    
    Use the opportunity to add some trivial smoke tests

commit 9f0cd745d07e7e379446eb82bcfd25e88e6d1d4a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Apr 10 11:41:54 2020 +0200

    Add a model object based version of the journal writer test objects
    
    This refactors the various conversions happening in the journal writer tests in
    a single place.
    
    When developing this, I noticed that the journal writer would let Content data
    go through; Make sure we do not let it do that.

commit 1c9ccb29eea3b974ea0d3613d61a65cd4e83037b
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 9 18:50:18 2020 +0200

    Rename OBJECT_TYPE_KEYS to TEST_OBJECT_DICTS

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

Harbormaster completed remote builds in B11785: Diff 10667.Apr 10 2020, 1:10 PM

olasd added a child revision: D3006: Ensure all object keys are passed through unharmed by the journal.Apr 10 2020, 1:16 PM

Mmm, i just realized you only moved stuff around ;)

swh/journal/serializers.py
24	Isn't model.BaseModel enough?
39	Content could be in the previous overload signature.
49	...Union[Origin, SkippedContent]... ?

This revision is now accepted and ready to land.Apr 10 2020, 4:00 PM

vlorentz added a subscriber: vlorentz.Apr 10 2020, 4:16 PM

vlorentz added inline comments.

swh/journal/serializers.py
24	Not exactly; there might be other subclasses. But I'm surprised `BaseModel` doesn't work though

Apply @ardumont's comments

Build is green

Patch application report for D3005 (id=10686)

Rebasing onto 9f0cd745d0...

Current branch diff-target is up to date.

Changes applied before test

commit cc5797df4b08c2493155997fcde85e9155e18e31
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 9 17:58:40 2020 +0200

    Move _get_key from the kafka writer to serializers
    
    Use the opportunity to add some trivial smoke tests

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

Harbormaster completed remote builds in B11807: Diff 10686.Apr 10 2020, 4:36 PM

Closed by commit rDJNLcc5797df4b08: Move _get_key from the kafka writer to serializers (authored by olasd). · Explain WhyApr 10 2020, 5:03 PM

This revision was automatically updated to reflect the committed changes.

olasd added a commit: rDJNLcc5797df4b08: Move _get_key from the kafka writer to serializers.