Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[REVIEW]: kiwiPy: Robust, high-volume, messaging for big-data and computational science workflows #2351
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @dghoshal-lbl, @uellue it looks like you're currently assigned to review this paper Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post. If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews To fix this do the following two things:
For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
If you see small problems that need to be discussed, feel free to discuss them here. But if you can, create a new issue in the target repository and link to this review thread in that issue to create a corresponding breadcrumb trail here. I look forward to seeing how this review goes |
Regarding "State of the field": Comparison with PikaAfter a quick side-by-side comparison between the RabbitMQ documentation and kiwiPy, it is not clear to me what the difference between the default Pika Python API and the kiwiPy API is. The code of the Pika-based RabbitMQ examples and the kiwiPy examles are quite similar, and at least to my eyes the kiwiPy version is not a noticeable improvement, different from what is claimed in the paper. (edit: source code formatting) Pika https://www.rabbitmq.com/tutorials/tutorial-two-python.html#!/usr/bin/env python
import time
import pika
connection = pika.BlockingConnection(
pika.ConnectionParameters(host='localhost'))
channel = connection.channel()
channel.queue_declare(queue='hello')
def callback(ch, method, properties, body):
print(" [x] Received %r" % body)
time.sleep(body.count(b'.'))
print(" [x] Done")
channel.basic_consume(
queue='hello', on_message_callback=callback, auto_ack=True)
print(' [*] Waiting for messages. To exit press CTRL+C')
channel.start_consuming() kiwiPy https://github.com/aiidateam/kiwipy/blob/develop/README.rst# -*- coding: utf-8 -*-
import time
import threading
import kiwipy
print(' [*] Waiting for messages. To exit press CTRL+C')
def callback(_comm, task):
print((' [x] Received %r' % task))
time.sleep(task.count('.'))
print(' [x] Done')
return task
with kiwipy.connect('amqp://127.0.0.1/') as communicator:
communicator.add_task_subscriber(callback)
try:
threading.Event().wait()
except KeyboardInterrupt:
pass Comparison with other work distribution and messaging systemsA quick list of major distributed work scheduling systems with Python API that might deserve a comparison:
The paper and documentation should make much clearer what makes kiwiPy unique and for what particular use cases it is advantageous. Perhaps it can also reference the appropriate parts of RabbitMQ documentation since it is essentially a Python API for it, as far as I understood. |
Regarding "References": A reference to the default "Pika" Python API for RabbitMQ https://pika.readthedocs.io/en/stable/ seems to be missing. |
Regarding "Community guidelines": aiidateam/kiwipy#65 |
@danielskatz @muhrin This concludes my first round of review. Looking forward to your responses! |
@uellue, many thanks for the helpful review. I'll coordinate with my coauthor and address the points you've raised. |
|
We've addressed @uellue issue regarding contributor guidelines by adding wiki entry with is now linked to in the docs. |
Dear @danielskatz, we once again thank @uellue for his thorough review which has helped to improve both the documentation and the paper itself. We have made the following changes to address the comments made:
This concludes our changes in response to the review. We look forward to the continuation of the review process. |
Dear @muhrin, many thanks for the update! The new content is a great help for readers to understand the use cases and strengths of kiwiPy. The comparison with Pika and other systems sounds fair and objective. CC @danielskatz |
Thanks @uellue . Indeed, the library has certainly benefited from these additions. Thanks again for your review. |
Happy to hear that you found it helpful! :-) |
|
I finished my first round of review for the software and the paper. Some of the paper related concerns have already been addressed. I have a couple of more questions regarding the paper: i) Has the library been tested to show any significant performance overheads? Mainly because this adds another level of abstraction to RabbitMQ. ii) Since one of the use-cases for kiwiPy is to restart partially completed workflows and HPC jobs, I was wondering if there are any specific configurations in kiwiPy that enables this or if this is a default behavior? Regarding the software, I was able to install it but not test its functionality properly: i) I could not find any module or scripts that would let me test the library. ii) When I wrote my own client and server following the examples, I ran into the following error: I also see that there are a few examples in the source tree. I can dig deeper into why I am getting this error, but I think it will be good to document the steps to quickly test the examples or provide scripts for validating the tests. |
|
whedon commentedJun 17, 2020
•
edited by dghoshal-lbl
Submitting author: @muhrin (Martin Uhrin)
Repository: https://github.com/aiidateam/kiwipy.git
Version: v0.6.0
Editor: @danielskatz
Reviewer: @dghoshal-lbl, @uellue
Archive: Pending
Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@dghoshal-lbl & @uellue, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @danielskatz know.
Review checklist for @dghoshal-lbl
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @uellue
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper