Skip to content
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

Merged
merged 11 commits into from
Jun 4, 2024
Merged

Conversation

ashum68
Copy link
Contributor

@ashum68 ashum68 commented May 29, 2024

No description provided.

Copy link
Contributor

@TongguangZhang TongguangZhang left a 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 @@
"""
Copy link
Contributor

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

@@ -0,0 +1,54 @@
"""
Worker Queue
Copy link
Contributor

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

Comment on lines 43 to 46
try:
self.queue.get(timeout=timeout)
except queue.Empty:
return
Copy link
Contributor

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

Copy link
Contributor

@TongguangZhang TongguangZhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed

timeout = self.__QUEUE_TIMEOUT

try:
self.queue.put(None, timeout=timeout)
Copy link
Contributor

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


try:
self.queue.put(None, timeout=timeout)
for _ in range(1, self.max_size):
Copy link
Contributor

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

Comment on lines 44 to 45
self.queue.get(timeout=timeout)
for _ in range(1, self.max_size):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Contributor

@TongguangZhang TongguangZhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ashum68 ashum68 merged commit a88cc8f into main Jun 4, 2024
1 check passed
@ashum68 ashum68 deleted the mp_workers branch June 4, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants