feat: introduce send_on_commit and send_async helpers in send_event()#570
feat: introduce send_on_commit and send_async helpers in send_event()#570bradenmacdonald wants to merge 2 commits intoopenedx:mainfrom
Conversation
|
Thanks for the pull request, @bradenmacdonald! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
08d3bfc to
101b83a
Compare
Co-Authored-By: Claude <noreply@anthropic.com>
101b83a to
9855182
Compare
Description
When sending events, two very common needs are to send events only when the transaction commits, and to send events asynchronously, to avoid slowdowns in the current user's request processing. Celery even has a
delay_on_commithelper for combining these two common requirements.However, this events library doesn't yet provide any way to do those things, so they're done very inconsistently at the moment.
Sending events asynchronously is a particular pain at the moment, because most event data structures use
attrsobjects which cannot be serialized to JSON, so you can't directly pass event data to a celery task. This PR solves that by using our existing event serialization mechanisms for celery as well as for event bus serialization.Other information
Although an
on_commithelper was mentioned in one of the ADRs, it was never implemented AFAIK:openedx-events/docs/decisions/0015-outbox-pattern-and-production-modes.rst
Lines 44 to 54 in b1d710a
This PR essentially implements both of the features mentioned in the ADR, although the new
send_asyncparameter does not provide the strong ordering guarantees of the proposed "outbox" mode.Things I didn't do
It would be awesome to have per-transaction event de-duplication; that is, if your application emits many identical events within a single database transaction using
send_on_commit=True, when the transaction commits, only a single event should be emitted. This would make many common content patterns more efficient without requiring any additional work on the part of event emitters.Ordering of async events - when you use
send_async=True, the events may end up getting emitted out of order. Except in test cases, because in tests you generally haveCELERY_ALWAYS_EAGER=Truewhich sends them synchronously anyways. The ordering could always be made guaranteed later on.Testing instructions
Here is a simple way to manually send all the different event combinations, using a shell like
./manage.py cms shell:Note that the error message "Received invalid content object id" is expected to be returned/printed from the event receiver, since we're passing "foo" as an event parameter.
Deadline
No particular deadline, though it would be nice to use this functionality in some of the event refactors I'm working on for Verawood.
Checklists
Check off if complete or not applicable:
Merge Checklist:
Post Merge:
finished.