-
Notifications
You must be signed in to change notification settings - Fork 11
fix: For the openedx-events broker, use kafka _everywhere_ instead of redis #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| EVENT_BUS_KAFKA_SCHEMA_REGISTRY_URL = 'http://edx.devstack.schema-registry:8081' | ||
| EVENT_BUS_KAFKA_BOOTSTRAP_SERVERS = 'edx.devstack.kafka:29092' | ||
| EVENT_BUS_PRODUCER = 'edx_event_bus_kafka.create_producer' | ||
| EVENT_BUS_CONSUMER = 'edx_event_bus_kafka.KafkaEventConsumer' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥳
| - ${DEVSTACK_WORKSPACE}/src:/edx/src | ||
| - ${PWD}/py_configuration_files/cms.py:/edx/app/edxapp/edx-platform/cms/envs/devstack.py | ||
| - ${PWD}/py_configuration_files/lms.py:/edx/app/edxapp/edx-platform/lms/envs/devstack.py | ||
| - ${PWD}/py_private_requirements/cms.txt:/edx/private_requirements.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[curious] Are there no private_requirements.txt for the LMS?
For example, is there an equivalent like the following needed:
${PWD}/py_private_requirements/lms.txt:/edx/private_requirements.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I don't know why settings are installed for both cms and lms for both images. But, in this case for requirements it would be redundant because they go to the same destination path inside the container.
| lms: | ||
| # Switch to `--settings devstack_with_worker` if you want to use lms-worker | ||
| command: bash -c 'source /edx/app/edxapp/edxapp_env && while true; do python /edx/app/edxapp/edx-platform/manage.py lms runserver 0.0.0.0:18000 --settings devstack; sleep 2; done' | ||
| command: bash -c 'source /edx/app/edxapp/edxapp_env && (pip install -r /edx/private_requirements.txt; while true; do python /edx/app/edxapp/edx-platform/manage.py lms runserver 0.0.0.0:18000 --settings devstack; sleep 2; done)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semi-related to my comment/question around LMS private_requirements.txt above; just trying to understand why these commands need to include pip install -r /edx/private_requirements.txt now when they didn't before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are several loosely related facts which are required to understand the situation:
- openedx-events is architected to support either Redis or Kafka as the message broker, and the docs recommend to use the same in both your prod and dev environments.
- Redis python libraries are already installed in many/most of our django apps, including edx-platform.
- Kafka python libraries are NOT installed in openedx/edx-platform, and it seems like Axim wants to keep it that way.
- For the 2U edx.org prod environment, Kafka was chosen as the platform-wide message broker. As such, we have custom python requirements [1, 2, 3] installed during our edxapp image build process to supplement the default requirements from
production.txt. - Devstack also used to be an openedx repo, and could not contain references to Kafka, therefore by default used the Redis broker. This is no longer a constraint because we forked Devstack.
- enterprise-subsidy in devstack was configured to use Kafka, and never successfully consumed events produced by edxapp (until this PR).
So, basically I needed a way to install extra Kafka python requirements into the edxapp container without modifying the edxapp image. The images Devstack pulls are all the standard openedx images published to docker hub, and there's no pre-existing pipeline to build 2u-devstack-specific images, so adding a docker-compose step to install additional requirements seemed appropriate.
aa2b45c to
7a6d393
Compare
|
Updated PR to also switch |
… redis Devstack is currently split between services pre-configured to use redis, and others pre-configured to use kafka. This results in the exceedingly dumb result of events produced via one broker, but never consumed because the consuming apps are hooked up to the other broker.
7a6d393 to
ae908da
Compare
Devstack is currently split between services pre-configured to use redis, and others pre-configured to use kafka. This results in the exceedingly dumb result of events produced via one broker, but never consumed because the consuming apps are hooked up to the other broker.
Just use the Kafka broker everywhere. Why Kafka? This is more consistent with our 2U/edx.org deployment.