-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: make use of abstract consumer API #13
Conversation
Thanks for the pull request, @navinkarkera! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
86fad2e
to
c71ff80
Compare
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.
👍
- I tested this: Followed testing instruction
- I read through the code
- I checked for accessibility issues
- Includes documentation
- I made sure any change in configuration variables is reflected in the corresponding client's
configuration-secure
repository.
Hi @navinkarkera! Is this PR ready for review/merge? |
@mphilbrick211 Thanks for checking. This MR is dependent on openedx/openedx-events#200, so I'll update and ping once it is ready. |
6b1505f
to
c850435
Compare
@bmtcril This is ready to be merged. We will need to release our first version to pypi in order to make use of this library in openedx and discovery. PS: I have also made the consumer argument required. |
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.
Thanks, looks good! I'll make a release now 🤞
@navinkarkera 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
@bmtcril Thanks!! |
Description: Removes local
consume_events
command and instead makes use of new abstract consumer API and the command from openedx-events to start a worker.JIRA:
Private-ref
: BB-7240Issue: openedx/openedx-events#147
Dependencies: openedx/openedx-events#200
Merge deadline: List merge deadline (if any)
Installation instructions: and Testing instructions:
Follow instructions from readme. The
consume_events
command is provided by openedx-events and theRedisEventConsumer
is loaded fromtest_settings.EVENT_BUS_CONSUMER
.Reviewers:
Merge checklist:
Post merge:
finished.
Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.