[Abstraction] Determine a dependency versioning strategy for event-bus implementations

[timmc-edx]

The edx_event_bus_kafka dependency will be specified and pinned in a file outside of each IDA that uses it, since openedx-events will load the implementation by looking up a module reference in the IDA's config. This means that dependency version changes on the library will not be versioned with the IDA (i.e. in the requirements/base.txt files) and we'll need some sort of expand-contract approach to API changes.

For example, we might need to do something like this:

1. Add a new KafkaProducer class in event-bus-kafka and delegate the existing send_to_event_bus to use it
2. Switch the producer code in Studio to use the new class
3. Delete the send_to_event_bus function from event-bus-kafka

Every master-branch deployer would need to do this in lockstep. (Maybe that's just 2U?) For Open edX releases we might be able to get away with just saying "use the release tag".

Acceptance Criteria

• Design a strategy that will work for deployers in general
• Document how-to for these upgrades in appropriate places.

[timmc-edx]

It turns out we have a collection of related dependency-related issues:

• A bunch of our repos will depend on event-bus-kafka but won't be able to declare it directly, and event-bus-kafka has its own dependencies that might cause deploy-time version conflicts.
  • As Robert notes, we won't be able to use make upgrade to keep things updated or synchronized (unless we change how deployments work).
• We'll have to do expand-contract deploys with event-bus-kafka as a result, and won't have safeguards. Maybe the same for all other deployers, eventually.
• There's a weird sort-of-bidirectional dependency between openedx-events and event-bus-kafka: The second declares a dependency on the first, but the first has an interface dependency on the second.

One thought I had is that we might just want to pack event-bus-kafka into openedx-events as a PEP-508 "extras" package, so we can include openedx-events[kafka] in base.in. (And eventually have an extra for each implementation.) Or maybe rewrite the dependency files to add the [kafka] part at AMI build time? It doesn't solve all the problems, but it would at least allow the interface to be versioned.

(You could even have a special CI workflow file on IDAs that confirms that none of the various extras' dependencies would conflict with the IDA's compiled requirements files.)

[robrap]

@timmc-edx: As noted elsewhere, I don't know enough about "extras", but am hopeful that maybe this could help simplify things? I don't really know though. It might simplify/avoid some cases of expand-contract, but all implementations of the interface would need to be updated at once. And that would only work if we made a decision that all implementations must be "extras", and that no one could create implementations in other repos or beware of broken contracts.

One point of clarification, is that I personally wouldn't refer to the relationship between and interface and its concrete implementation as a "weird sort-of-bidirectional dependency", unless I'm missing something. The concrete implementation is definitely dependent on the interface. The interface is not directly dependent on the implementation, but it needs to know and respect that it is a contract with a set of unknown (or possibly known) consumers. I think this just comes with the territory of loose coupling. It can certainly make things messy or interesting, which might be what you meant by "weird"? I'm simply noting that it is expected, and I don't think we are doing anything out of the ordinary.

[timmc-edx]

What's feeling weird about it is that the implementation has a package-dependency on the interface package, but the interface package loads the implementation. But... just a feeling. I haven't thought it all through yet, though, so maybe there's no actual additional complication resulting from that.

[ormsbee]

What's feeling weird about it is that the implementation has a package-dependency on the interface package, but the interface package loads the implementation. But... just a feeling. I haven't thought it all through yet, though, so maybe there's no actual additional complication resulting from that.

Could you please point me to where that loading happens? Does it do something other than receive Django signals?

It might simplify/avoid some cases of expand-contract, but all implementations of the interface would need to be updated at once. And that would only work if we made a decision that all implementations must be "extras", and that no one could create implementations in other repos or beware of broken contracts.

I don't think I fully understand the issues at play here, but would it simplify things any to always install the kafka dependencies? We could keep the entire package separated at the top level (so it'd be in its own package as a peer to openedx_events). That way we could still keep some level of separation that would make it easy to extract later, while simplifying development in these relatively early days?

[robrap]

The idea is for openedx-events to contain event bus related APIs that producers and consumers could depend on, such that the Kafka implementation (or any other implementation) is selected via configuration settings. Thus, an Open edX deployment that is not using Kafka would not pull in the Kafka dependency.

This also entails having a way for anyone to privately install the Kafka dependency, which for now was being done in our private Dockerfile.

As you point out, a quick solution would be to simply install the Kafka dependencies for everyone. This could be done now by adding our Kafka dependency (event-bus-kafka) in the requirements files of the producing or consuming service. The Kafka code could continue to live in a separate repo. This would be a decision to temporarily punt this issue down the road.

[ormsbee]

@felipemontoya and @mariajgrimaldi should weigh in, but I'm personally okay with Kafka dependencies directly in this repo in the short term to get the first iteration out the door, so long as it's very clear how to separate it into a new repo later (e.g. different top level package).

Please forgive my ignorance here, but what common APIs are necessary across the different bus backends aside from sending/receiving signals?

[robrap]

Thanks @ormsbee.

1. It may be that edx-platform (and discovery, etc.) gets dependencies of both openedx-events and event-bus-kafka, rather than openedx-events getting Kafka dependencies, but we need to discuss what would be most efficient for us at this point.
2. The common APIs are in flux and something that we've realized we need to defer until we have a more complete version of code working to iterate upon. One example is the the consumer management command that listens for new events and sends the consumer-side signals. The shape of the API and reusable code is in flux, including what code should be in a library vs code that gets duplicated.

[robrap]

@timmc-edx: If we decide to simply make Kafka a dependency, does adding event-bus-kafka as a dependency to edx-platform resolve enough, or do you think we need to drop the separate repo altogether? I'm thinking we start with the small simple fix of just adding the dependency, and only move all code out of event-bus-kafka if we think we are still being slowed down by having a separate repo.

[felipemontoya]

I'd like to make sure I understand what adding the kafka dependency here means.

Is it to simply add confluent-kafka and maybe a few others in this repo? Or would that mean merging the code from openedx/event-bus-kafka to this repo?

If it is the first I would personally not mind it either. If the "few others" are all the dependencies from the event-bus-kafka I suppose that would be ok to get this off the pad but in that case I would also advocate for using the extras to keep the package lightweight for other uses in plugin development.

[robrap]

@felipemontoya: See my responses above. I am proposing that nothing changes for openedx-events in the short term.

[mariajgrimaldi]

Extra dependencies, in this case, seem quite handy. But it seems like a good idea if there's some urgency in the issue to include the dependency in the requirements files of the producing or consuming service. I still wonder how to handle conflicting versions of openedx-events inside those services, but it seems an issue that could be tackled down the road (documentation surrounding this should be crucial).

[timmc-edx]

@robrap Adding edx_event_bus_kafka as an explicit dependency (requirements/base.in) of any producing or consuming IDA would solve this issue, yes. And it would avoid all the problems the "extras" approach would itself introduce.

There are still some issues to resolve around how we'll coordinate API change management, but I think we can figure that out as needed. (We can just do major-version bumps of openedx-events and event-bus-kafka in lockstep, and if that's too rigid we can do something more clever.)

[robrap]

Adding the dependencies in the service also means we can use the constraints file to manage rollouts as required.

[dianakhuang]

We had an issue earlier where confluent-kafka relies on a package that did not have a build for ARM64: openedx/edx-platform#29838 (comment)

This has been discussed with the confluent devs: confluentinc/confluent-kafka-python#1405

There has been a wheel published for M1 Macs/ARM64: https://github.com/confluentinc/confluent-kafka-python/releases/tag/v1.9.2

[timmc-edx]

I think what we want at this point is just an ADR in openedx-events saying that we're going to depend on all Event Bus implementations for now and will figure out a better approach later, if needed. (event-bus-kafka no longer has a hard dependency on confluent-kafka as far as pip is concerned: https://github.com/openedx/event-bus-kafka/blob/main/docs/decisions/0005-optional-import-of-confluent-kafka.rst)

[robrap]

I'm actually confused about the current status. That ADR does not have what I remember. :) I thought we were getting upgrades via make upgrade, but I don't see where we are requiring it other than tests, and the weird enterprise constraint. See https://github.com/search?p=1&q=org%3Aopenedx+confluent-kafka&type=Code.

1. I'd like to get clear on where the dependency is currently being required from Studio and Discovery.
2. Based on this, we can determine the best place for an ADR (which might match your recommendation), and potentially update the existing how-to?
3. I think with each new technology, it may make sense to revisit this issue, rather than jumping directly to adding dependencies for ALL technologies. However, I'm not clear on how it is working right now. :)

[timmc-edx]

Currently it is pulled in here, in event-bus-kafka test.in -- but not by the package's base.in: https://github.com/openedx/event-bus-kafka/blob/701098b0b7a671982b8c74855afd13c04ef81612/requirements/test.in#L9 (note that it's spelled with an underscore here, which is why it wasn't coming up in your search -- we should probably switch it to a hyphen to match its canonical spelling!)

So event-bus-kafka doesn't pull it in as a dependency, but will just be a no-op if you try to use it. CMS and course discovery have to install confluent-kafka in their private dockerfile or something.

[robrap]

The part I'm confused about is I thought we determined that upgrades weren't going to be issue because we were using a public dependency from the services and make upgrade would pull in the latest. But now I understand that that is not the case. Maybe I had just forgotten.

So this ticket, or something related to this ticket, still seems like it applies, right?

[timmc-edx]

At the time this ticket was written, edx-event-bus-kafka was a private dependency. But that's no longer true -- it's confluent-kafka that's a private dependency. So the ticket is now incorrect.

[timmc-edx]

Created edx/edx-arch-experiments#351 for the general case of "we don't know how to manage plugin versions yet".