openedx / openedx-events

Open edX events from the Hooks Extensions Framework
Apache License 2.0
11 stars 21 forks source link

fix: avoid infinite recursion of openedx-event #312

Closed Ian2012 closed 7 months ago

Ian2012 commented 7 months ago

Description: This PR adds a new argument from_event_bus to the send_event method (and related ones) that allows marking a Django signal as already processed from the event bus and avoids infinite emission of the same signal if such signal is consumed in the same runtime.

The following code block must be used on the consumer to avoid processing the signal in the same process that emitted the signal:

from openedx_events.tooling import SIGNAL_PROCESSED_FROM_EVENT_BUS

...

def my_signal(...):
    if not kwargs.get(SIGNAL_PROCESSED_FROM_EVENT_BUS, False):
        logger.info("Event received from a non-event bus backend, skipping...")
        return

This has been tested with the event-bus-redis without any changes as the behavior of calling send_event_with_custom_metadata is to enable the from_event_bus param. Tested on https://github.com/openedx/event-routing-backends/pull/361 with the signal emitted on https://github.com/openedx/event-tracking/pull/246

ISSUE: Solves: https://github.com/openedx/openedx-events/issues/79

Testing instructions:

Merge checklist:

Post merge:

Author concerns: List any concerns about this PR - inelegant solutions, hacks, quick-and-dirty implementations, concerns about migrations, etc.

openedx-webhooks commented 7 months ago

Thanks for the pull request, @Ian2012! 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.

mariajgrimaldi commented 7 months ago

The next release with these changes will affect the event bus implementation tooling, so I'm pinging @robrap @timmc-edx so they know. Thanks!

Ian2012 commented 7 months ago

The next release with these changes will affect the event bus implementation tooling, so I'm pinging @robrap @timmc-edx so they know. Thanks!

@mariajgrimaldi it will not affect them as event-bus-redis and event-bus-kafka already call send_event_with_custom_metadata on receiving signals from the event bus. There doesn't exist a signal that is produced/consumed on the same runtime (except this current work: https://github.com/openedx/event-routing-backends/pull/361) and this PR makes the default behavior to mark the signal when calling such a method.

Ian2012 commented 7 months ago

@timmc-edx rebased and bumped. Do you think is ready to be merged? cc @mariajgrimaldi

openedx-webhooks commented 7 months ago

@Ian2012 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.