Details
- Reviewers
olasd ardumont douardda - Group Reviewers
Reviewers - Commits
- rDJNL33e79ab2d613: Add a draft of 'StorageReplayer', a tool that fills a Storage from Kafka.
Diff Detail
- Repository
- rDJNL Journal infrastructure
- Branch
- add-StorageReplayer
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 5008 Build 6720: tox-on-jenkins Jenkins Build 6719: arc lint + arc unit
Event Timeline
Build has FAILED
Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tox/75/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tox/75/console
Build is green
See https://jenkins.softwareheritage.org/job/DJNL/job/tox/77/ for more details.
Build is green
See https://jenkins.softwareheritage.org/job/DJNL/job/tox/78/ for more details.
swh/journal/replay.py | ||
---|---|---|
23–40 | why not pass the kafka consumer instance as argument of the constructor instead of passing KafkaConsumer init arguments? | |
44 | I find a bit odd to put the enumerate inside the islice iterator. It makes it a bit harder to read this line IMHO. Also, since you have to initialize num=0 to handle empty look, why not add a num += 1 in the for loop instead of using enumerate? | |
58 | This needs an explanation (comment) | |
66 | This is a bit rough |
- Apply @ddouard's comments.
swh/journal/replay.py | ||
---|---|---|
23–40 | For consistency with other related classes (eg. KafkaDirectWriter), whose arguments are deserialized directly from a yaml. | |
44 | Indeed, I hesitated between the two approach so I unknowingly did both | |
66 | It can't happen (because of the assertion in the other method), but I don't like my pattern-matchings to be non-exhaustive. |
Build has FAILED
Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tox/79/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tox/79/console
Build is green
See https://jenkins.softwareheritage.org/job/DJNL/job/tox/80/ for more details.
swh/journal/replay.py | ||
---|---|---|
66 | ok, then raising a RuntimeError (or any suitable exception) would be preferrable IMHO. |
swh/journal/replay.py | ||
---|---|---|
66 | Why? That's what assertions are for |
swh/journal/replay.py | ||
---|---|---|
66 | well I guess so. Not a huge fan of this usage (but meh). |
Build is green
See https://jenkins.softwareheritage.org/job/DJNL/job/tox/82/ for more details.