feat: claim pending messages from other consumers #20
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. |
openedx-webhooks
added
the
open-source-contribution
| db: Walrus object for redis connection. | ||
| full_topic: topic prefixed with environment name. | ||
| consumer: consumer instance. | ||
| """ | ||
|
|
||
| def __init__(self, topic, group_id, signal, consumer_name, last_read_msg_id=None, check_backlog=False): | ||
| def __init__(self, topic, group_id, signal, consumer_name, last_read_msg_id=None, check_backlog=False, | ||
| claim_msgs_older_than=None): |
There was a problem hiding this comment.
Is this flag documented anywhere? It seems like the default behavior will be to never claim, which seems wrong.
There was a problem hiding this comment.
(Not necessarily wrong that it won't claim messages, but that there's no doc explaining that you probably really do want this set to some other value. I think a sensible default would be somewhere in the range of a minute if we wanted to add one, since it needs to be long enough that an long-running jobs won't accidentally get double processed.
There was a problem hiding this comment.
@bmtcril It is documented above in line 96. Do you have a suggestion for a default idle time? This behavior is similar to kafka event bus by default as it also doesn't claim messages from other consumers.
There was a problem hiding this comment.
Hmmm I meant more in a "as a site operator deploying this, how would I know what this flag is and what to set it to", but it actually looks like we don't have any of that kind of documentation right now? I think it's unrelated to this PR, but would be good to provide a doc of how to run in production as well as a list of all of the settings and command line options in the docs.
There was a problem hiding this comment.
Ahh we commented at the same time. We could add some examples along with the tutor/devstack docs that we are working on in the other ticket explaining all the flags and their default values.
Co-authored-by: Maxim Beder <30300520+Cup0fCoffee@users.noreply.github.com>
navinkarkera
force-pushed
the
navin/claim-msgs
branch
from
20162bb to
2745ee1
Compare
|
@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. |
Mirror of open-craft#1 as github does not allow changing PRs across forks.