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.

Sign up for GitHub

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

Jump to bottom

PoC: Redis List for single consumer use case #365

Closed
wants to merge 2 commits into from
Closed

PoC: Redis List for single consumer use case #365

wants to merge 2 commits into from

Conversation

JenySadadia
Copy link
Collaborator

@JenySadadia JenySadadia commented Sep 28, 2023 •
edited
Loading

Integrated redis package.
Used Redis List data structure for single consumer use case instead of broadcasting messages with Redis PubSub.

Jeny Sadadia added 2 commits September 28, 2023 17:29
docker/api: use redis package
25571ad 
Drop `aioredis` package as the project has
been archived and it is now a part of `redis`
package. Use `redis` directly.

Signed-off-by: Jeny Sadadia <[email protected]>
api.pubsub: integrate Redis List
4d806a8 
Instead of broadcasting all the messages through
Redis PubSub, use `Redis List` data structure
to manage a queue of messages.
`redis.lpush` is used to push message on a specific
list in `publish` handler.
`redis.blpop` is used for blocking pop operation in
`listen` handler.

Signed-off-by: Jeny Sadadia <[email protected]>
@JenySadadia JenySadadia marked this pull request as ready for review September 28, 2023 13:19
@JenySadadia
Copy link
Collaborator Author

I have tested this using kci pubsub commands.
Please use changes from kernelci/kernelci-core#2118 for testing.

@JenySadadia
Copy link
Collaborator Author

@gctucker As this is just a PoC, I have updated existing methods for publishing and listening messages of PubSub to avoid introducing different API endpoints for single consumer use case atm.

@JenySadadia JenySadadia mentioned this pull request Sep 28, 2023
Open
@JenySadadia JenySadadia added the staging-skip Don't test automatically on staging.kernelci.org label Sep 29, 2023
@gctucker
Copy link
Contributor

OK I see. Please just don't forget to set staging-skip labels on all the PRs such as this one so it doesn't break staging etc.

Then I guess this PR will just be closed and not merged once the experiment is complete, or updated with a final version that can be merged? Maybe this should be a draft PR too.

@JenySadadia JenySadadia marked this pull request as draft September 29, 2023 09:55
@JenySadadia
Copy link
Collaborator Author

OK I see. Please just don't forget to set staging-skip labels on all the PRs such as this one so it doesn't break staging etc.

Then I guess this PR will just be closed and not merged once the experiment is complete, or updated with a final version that can be merged? Maybe this should be a draft PR too.

Converted both the PRs to drafts.

@JenySadadia
Copy link
Collaborator Author

Closing the PR as it was just a PoC for #417.

@JenySadadia JenySadadia closed this Dec 8, 2023
@JenySadadia JenySadadia deleted the redis-list-poc branch December 19, 2023 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gctucker gctucker Awaiting requested review from gctucker

Assignees
No one assigned
Labels
staging-skip Don't test automatically on staging.kernelci.org
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JenySadadia @gctucker