Also drop implicit config from JournalClient.
It might fix T1513, but not sure.
Details
Details
Diff Detail
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
Comment Actions
Build is green
See https://jenkins.softwareheritage.org/job/DJNL/job/tox/149/ for more details.
Comment Actions
Finish handling polled message after max_message is reached,
so we don't commit un-handled messages.
Comment Actions
Build is green
See https://jenkins.softwareheritage.org/job/DJNL/job/tox/150/ for more details.
Comment Actions
I am not very happy with the API of the JournalClient (it's not the result of this diff, but since we are discussing refactorings in there...)
I don't like to be obliged to use inheritance to, in fact, only define a callback. Why not pass this as a callable argument to JournalClient.process()?
Doing so, the StorageReplayer class is not needed anymore and insert_objects becomes a simple function (and JournalClient must not be abstract any more either).
| swh/journal/replay.py | ||
|---|---|---|
| 31–52 | This code could also be simplified. At least there is no need for keeping the 'content' in the list of checked object type in the first if statement since the first statement in the block is if object_type == 'content': I mean this should be enough: if object_type == 'content': # do stuff elif object_type == 'origin_visit': # do stuff elif content_type in (remaining_types): # do stuff else: raise ValueError() This could also be implemeted as a visitor, but meh. | |
| swh/journal/client.py | ||
|---|---|---|
| 87–91 | In fact I don't really see the advantage of implementing the max_messages behavior in there. The process should be a process_one_poll (or so) and let the caller handle the max messages logic. | |
Comment Actions
Build is green
See https://jenkins.softwareheritage.org/job/DJNL/job/tox/151/ for more details.