-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
created multiprocessing worker controller and queue #4
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed, make sure to add an empty init.py file to the workers folder, this sets it up as a module to be imported
main.py
Outdated
@@ -0,0 +1,70 @@ | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file should be part of a separate pr
worker/queue_wrapper.py
Outdated
@@ -0,0 +1,54 @@ | |||
""" | |||
Worker Queue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure you have periods at the end of docstrings
worker/queue_wrapper.py
Outdated
try: | ||
self.queue.get(timeout=timeout) | ||
except queue.Empty: | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this only removes 1 item from the queue, loop through the maxsize (like the fill function above) to completely drain the queue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed
worker/queue_wrapper.py
Outdated
timeout = self.__QUEUE_TIMEOUT | ||
|
||
try: | ||
self.queue.put(None, timeout=timeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need this line since it's covered below
worker/queue_wrapper.py
Outdated
|
||
try: | ||
self.queue.put(None, timeout=timeout) | ||
for _ in range(1, self.max_size): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start the range from 0 to cover above
worker/queue_wrapper.py
Outdated
self.queue.get(timeout=timeout) | ||
for _ in range(1, self.max_size): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
No description provided.