feat: add listener for tracking event emitted signal #305
Conversation
openedx-webhooks
added
the
open-source-contribution
|
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. This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate. |
Ian2012
force-pushed
the
cag/add-consumer-for-tracking-logs
branch
from
51656fa to
f420d7d
Compare
Ian2012
changed the title
Ian2012
force-pushed
the
cag/add-consumer-for-tracking-logs
branch
from
f420d7d to
f103064
Compare
| """ | ||
| event = kwargs['tracking_log'] | ||
|
|
||
| ## TODO: Should we ignore events that we don't care about here or in the event routing backend config? |
There was a problem hiding this comment.
@bmtcril Please read my comments
There was a problem hiding this comment.
It's possible to ignore events at the tracking log configuration level, which may be enough for this and is probably better for overall system performance.
| #if SEND_TRACKING_EVENT_EMITTED_SIGNAL.is_enabled(): | ||
| # get_producer().send( | ||
| # signal=TRACKING_EVENT_EMITTED, | ||
| # topic='analytics', | ||
| # event_key_field='tracking_log.name', | ||
| # event_data={'tracking_log': kwargs['tracking_log']}, | ||
| # event_metadata=kwargs['metadata'] | ||
| # ) |
There was a problem hiding this comment.
@bmtcril I guess we would need to write the code for the event-bus consumer, but how will we send queued events in batch to the event bus?
There was a problem hiding this comment.
I don't think there's any additional code needed for the consumer. Running the consume_events command inside a configured edx-platform container will read events off the event bus and fire off this signal for events that match. The batching capability is in my current PR: https://github.com/openedx/event-routing-backends/pull/301/files#diff-f24cf45168ff0c054647f2bf8c504fef13b9acf37fd97eedfd7410cc61c06458R112
You would need to create an EventsRouter with the processors defined in configuration just like the AsyncRouter does (note: not like the management command does, it does not use the processors). Then you can call bulk_send on that and it will find all of the configured LRSs and POST the batch to them.
robrap
added
the
waiting on author
|
Hi @Ian2012! Just checking in to see if you plan to pursue this pull request? |
mphilbrick211
added
the
inactive
mphilbrick211
added
closed inactivity
|
Closing this due to inactivity. |
|
@Ian2012 Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future. |
1 similar comment
|
@Ian2012 Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future. |
Description: Add consumer code for the
TRACKING_EVENT_EMITTEDsignal openedx/openedx-events#230Dependencies: openedx/openedx-events#230, openedx/event-tracking#225
Merge deadline:
Installation instructions: List any non-trivial installation
instructions.
Testing instructions:
Merge checklist:
Post merge:
finished.
Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.