docs: Add ADR for depending on multiple event bus implementations #227
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
docs/decisions/0011-depending-on-multiple-event-bus-implementations.rst
Outdated
Show resolved
Hide resolved
2 tasks
timmc-edx
commented
May 25, 2023
robrap
reviewed
Jun 1, 2023
| There are also some downsides: | ||
|
|
||
| - The dependency tree is larger, and is more or less guaranteed to include at least one top-level dependency that each deployer is not using. Not only does each dependency take up bandwidth to download, space on disk, and time spent in upgrading packages, but it can also bring licensing and architecture incompatibilities. | ||
| - More concretely, event-bus-kafka depends on confluent-kafka, but that package does not provide binaries compatible with Linux on Apple Silicon. This means that developers on Apple M1 laptops cannot install this package in devstack at all. So as a knock-on effect, event-bus-kafka has had to keep confluent-kafka out of its own in-tree dependencies; deployers wishing to use it have to install and version that package out-of-tree anyhow. While this may eventually be resolved, this does obviate a number of the stated benefits for development and version management for the Kafka users. |
There was a problem hiding this comment.
I thought that was something about this as "context" that made it especially painful when we were trying to install the packages optionally? Maybe I am misremembering? If you can recall, maybe calling that out as part of the context for making the decision would be useful. If not, please ignore.
There was a problem hiding this comment.
I think that we discovered the Apple Silicon problem after we had already started installing edx-event-bus-kafka in Ansible, so from the perspective of a somewhat retroactive ADR it's more Consequence than Context.
robrap
approved these changes
Jun 6, 2023
docs/decisions/0011-depending-on-multiple-event-bus-implementations.rst
Outdated
Show resolved
Hide resolved
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.
Issue: #209
Merge checklist: