feat: use new event_bus_producer_config #33458
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rgraber
force-pushed
the
rsgraber/20231004-upgrade-openedx-events-to-9
branch
2 times, most recently
from
41335a1 to
405780d
Compare
rgraber
force-pushed
the
rsgraber/20231004-upgrade-openedx-events-to-9
branch
from
50739ee to
fd482c6
Compare
robrap
reviewed
Oct 11, 2023
|
|
||
| Parameters | ||
| signal (OpenEdxPublicSignal): The signal being sent to the event bus | ||
| topic (string): The topic to which the signal is being sent |
There was a problem hiding this comment.
Can you document which topic names include or exclude the prefix, and ensure that everything is consistent?
rgraber
force-pushed
the
rsgraber/20231004-upgrade-openedx-events-to-9
branch
from
6e8c0c8 to
39deed0
Compare
timmc-edx
reviewed
Oct 12, 2023
timmc-edx
reviewed
Oct 12, 2023
timmc-edx
reviewed
Oct 12, 2023
timmc-edx
reviewed
Oct 12, 2023
timmc-edx
reviewed
Oct 12, 2023
timmc-edx
reviewed
Oct 12, 2023
rgraber
force-pushed
the
rsgraber/20231004-upgrade-openedx-events-to-9
branch
from
2828379 to
44d7cc9
Compare
rgraber
force-pushed
the
rsgraber/20231004-upgrade-openedx-events-to-9
branch
from
17966e8 to
b5c74f8
Compare
timmc-edx
approved these changes
Oct 16, 2023
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
|
2U Release Notice: This PR has been deployed to the edX production environment. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
|
2U Release Notice: This PR has been deployed to the edX production environment. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR replaces #32874 and #33269
Description
Update edx-platform to use the new event bus producer configuration format specified in [email protected]. In order to better control the rollout, we are doing an expand-contract. For this PR, all manual sends in the lms and cms will first check to see if the new configuration is present for the given event/topic combination. If the event/topic is enabled in the new configuration, the manual send will not occur. Otherwise the logic for the manual send will continue as before.
How we use the new config is a little complex in order to account for a few use cases:
enabledflags of the relevant events can be set according to the value of a single feature flag.merge_producer_configsfrom openedx-events to combine the base config and the yml config. This means that operators don't need to copy over the whole configuration from edx-platform to enable/disable a single event while keeping everything else the same as the base config.Supporting information
Roll out plan: https://openedx.atlassian.net/wiki/spaces/AC/pages/3886120980/Rollout+plan+for+publish-via-config
GH isssue: edx/edx-arch-experiments#381
Testing instructions
We can test by firing all the relevant events on stage and making sure nothing is duplicated or lost.
Tested locally to the best of my ability by trying to fire CMS events with combinations of both the old ENABLE_SEND_XBLOCK_EVENTS_OVER_BUS flag and the new config. I was unable to figure out how to fire the LMS events locally but am reasonably confident they will behave the same.
Deadline
"None" if there's no rush, or provide a specific date or event (and reason) if there is one.