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
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
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.