Skip to content

dev 1757 add imemoryqueue to queue factory#13

Merged
qqiao2024 merged 8 commits intodevelopfrom
feat/dev-1757-add-InMemoryQueueClient-to-QueueFactory
Mar 28, 2023
Merged

dev 1757 add imemoryqueue to queue factory#13
qqiao2024 merged 8 commits intodevelopfrom
feat/dev-1757-add-InMemoryQueueClient-to-QueueFactory

Conversation

@qqiao2024
Copy link
Copy Markdown

No description provided.

@qqiao2024 qqiao2024 marked this pull request as ready for review March 17, 2023 13:57
@qqiao2024 qqiao2024 changed the title add imemoryqueue to queue factory dev 1757 add imemoryqueue to queue factory Mar 20, 2023
Comment thread queueclient/core.py Outdated

class InMemoryQueueClient(QueueClient):

queues = {}
Copy link
Copy Markdown

@jiakf jiakf Mar 21, 2023

Choose a reason for hiding this comment

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

not sure if this is the best place for the holding and keeping track of queues, seems like ideally that responsibility would not be up to the client class. also, as an aside, if we used the the built in python Queue class instead of deque, the queues themselves would be threadsafe, although then we would probably want to guard the actual queues dictionary.

it also seems like we are changing the functionality of InMemoryQueueClient, as before a queue client could only connect to one queue. Did we need this added functionality somewhere else?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Moved to QueueFactory,
changed to multiprocessing Queue.
This InMemoryQueue should only be used for testing.

@qqiao2024 qqiao2024 requested a review from jiakf March 21, 2023 17:32
@@ -0,0 +1,53 @@
import os
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you use a git mv or did you delete the file and recreate it? It looks like it was recreated, so I would go back and restore the previous versions and them git mv to keep the history.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Just tried. After I add an empty init.py, it will come back to this.

@qqiao2024 qqiao2024 merged commit 2b44503 into develop Mar 28, 2023
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.

3 participants