Skip to content
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

[Abstraction] Configurable loading of Event Bus producer implementation #87

Closed
4 tasks done
timmc-edx opened this issue Aug 3, 2022 · 2 comments
Closed
4 tasks done
Assignees
Labels
event-bus Work related to the Event Bus.

Comments

@timmc-edx
Copy link
Contributor

timmc-edx commented Aug 3, 2022

We need a way to load event-bus implementations based on configuration, so that IDAs don't have to depend directly on those libraries.

This issue is to define and implement an interface that allows an IDA to retrieve a producer instance by looking at Django settings. Consumer side has been split off to #147 since it is lower priority.

@timmc-edx timmc-edx added event-bus Work related to the Event Bus. backlog To be put on a team's backlog or wishlist labels Aug 3, 2022
@timmc-edx timmc-edx moved this to Todo in Arch-BOM Aug 3, 2022
@robrap robrap added this to the [Event Bus] Future milestone Aug 4, 2022
@robrap robrap changed the title Configurable implementation-loader [Abstraction] Configurable implementation-loader Aug 9, 2022
timmc-edx added a commit to openedx/event-bus-kafka that referenced this issue Aug 26, 2022
Purpose:

- Revisit #16 since I
  finally figured out a clean way to have a single producer.
- Reduce the burden on future code that will need to adjust how polling
  is done (#31) and
  maybe handle shutdown (#11)
- Prepare for configurable implementation loading, which will need a
  singleton and getter: openedx/openedx-events#87
- Get rid of the `sync` argument (which didn't fit the abstraction) and
  move it to a dedicated method.

Relying code should now call `get_producer().send(...)` rather than
`send_to_event_bus(...)`. The return value is an object that wraps a
`Producer` instance (not a `SerializingProducer`) and that handles the
serialization itself.

Serialization logic is moved to a cached `get_serializers(...)` that
expands upon the previous `get_serializer` function; it now returns a pair
of key and value serializers. This also acts as a patch point for mocking.

`send_to_event_bus` gets a shorter name (now it's just a `send` method)
and loses the `sync` keyword argument; there is instead now a
`pre_shutdown` method.
timmc-edx added a commit to openedx/event-bus-kafka that referenced this issue Aug 26, 2022
Purpose:

- Revisit #16 since I
  finally figured out a clean way to have a single producer.
- Reduce the burden on future code that will need to adjust how polling
  is done (#31) and
  maybe handle shutdown (#11)
- Prepare for configurable implementation loading, which will need a
  singleton and getter: openedx/openedx-events#87
- Get rid of the `sync` argument (which didn't fit the abstraction) and
  move it to a dedicated method.

Relying code should now call `get_producer().send(...)` rather than
`send_to_event_bus(...)`. The return value is an object that wraps a
`Producer` instance (not a `SerializingProducer`) and that handles the
serialization itself.

Serialization logic is moved to a cached `get_serializers(...)` that
expands upon the previous `get_serializer` function; it now returns a pair
of key and value serializers. This also acts as a patch point for mocking.

`send_to_event_bus` gets a shorter name (now it's just a `send` method)
and loses the `sync` keyword argument; there is instead now a
`pre_shutdown` method.
timmc-edx added a commit to openedx/event-bus-kafka that referenced this issue Aug 26, 2022
Purpose:

- Revisit #16 since I
  finally figured out a clean way to have a single producer.
- Reduce the burden on future code that will need to adjust how polling
  is done (#31) and
  maybe handle shutdown (#11)
- Prepare for configurable implementation loading, which will need a
  singleton and getter: openedx/openedx-events#87
- Get rid of the `sync` argument (which didn't fit the abstraction) and
  move it to a dedicated method.

Relying code should now call `get_producer().send(...)` rather than
`send_to_event_bus(...)`. The return value is an object that wraps a
`Producer` instance (not a `SerializingProducer`) and that handles the
serialization itself.

Serialization logic is moved to a cached `get_serializers(...)` that
expands upon the previous `get_serializer` function; it now returns a pair
of key and value serializers. This also acts as a patch point for mocking.

`send_to_event_bus` gets a shorter name (now it's just a `send` method)
and loses the `sync` keyword argument; there is instead now a
`pre_shutdown` method.
timmc-edx added a commit to openedx/event-bus-kafka that referenced this issue Aug 26, 2022
Purpose:

- Revisit #16 since I
  finally figured out a clean way to have a single producer.
- Reduce the burden on future code that will need to adjust how polling
  is done (#31) and
  maybe handle shutdown (#11)
- Prepare for configurable implementation loading, which will need a
  singleton and getter: openedx/openedx-events#87
- Get rid of the `sync` argument (which didn't fit the abstraction) and
  move it to a dedicated method.

Relying code should now call `get_producer().send(...)` rather than
`send_to_event_bus(...)`. The return value is an object that wraps a
`Producer` instance (not a `SerializingProducer`) and that handles the
serialization itself.

Serialization logic is moved to a cached `get_serializers(...)` that
expands upon the previous `get_serializer` function; it now returns a pair
of key and value serializers. This also acts as a patch point for
mocking. I'd like to test the serializers themselves, but they want to
talk to a server.

`send_to_event_bus` gets a shorter name (now it's just a `send` method)
and loses the `sync` keyword argument; there is instead now a
`pre_shutdown` method.
timmc-edx added a commit to openedx/event-bus-kafka that referenced this issue Aug 31, 2022
Purpose:

- Revisit #16 since I
  finally figured out a clean way to have a single producer.
- Reduce the burden on future code that will need to adjust how polling
  is done (#31) and
  maybe handle shutdown (#11)
- Prepare for configurable implementation loading, which will need a
  singleton and getter: openedx/openedx-events#87
- Get rid of the `sync` argument (which didn't fit the abstraction) and
  move it to a dedicated method.

Relying code should now call `get_producer().send(...)` rather than
`send_to_event_bus(...)`. The return value is an object that wraps a
`Producer` instance (not a `SerializingProducer`) and that handles the
serialization itself.

Serialization logic is moved to a cached `get_serializers(...)` that
expands upon the previous `get_serializer` function; it now returns a pair
of key and value serializers. This also acts as a patch point for
mocking. I'd like to test the serializers themselves, but they want to
talk to a server.

`send_to_event_bus` gets a shorter name (now it's just a `send` method)
and loses the `sync` keyword argument; there is instead now a
`prepare_for_shutdown` method.

Other refactoring:

- Cache `create_schema_registry_client` and rename to `get_...`
- Lift producer test data to be instance variables
@robrap
Copy link
Contributor

robrap commented Nov 18, 2022

We think this will be quick to land, so we will land it while our head is in it. If there are any surprises, we should defer.

@robrap robrap removed the backlog To be put on a team's backlog or wishlist label Nov 18, 2022
@robrap robrap moved this from Todo to Groomed in Arch-BOM Nov 18, 2022
@robrap
Copy link
Contributor

robrap commented Nov 18, 2022

@timmc-edx: I'm calling this groomed and assigned it to you, because I think our assumptions were based on you closing this out.

UPDATE: I moved to in-progress because you requested reviews on the PRs. I also added the PRs to the board.

@robrap robrap moved this from Groomed to In Progress in Arch-BOM Nov 18, 2022
@timmc-edx timmc-edx changed the title [Abstraction] Configurable implementation-loader [Abstraction] Configurable loading of Event Bus producer implementation Nov 21, 2022
timmc-edx added a commit to openedx/event-bus-kafka that referenced this issue Nov 21, 2022
Remove caching of `get_producer` since openedx-events will take care of
that for us.

This is part of openedx/openedx-events#87
timmc-edx added a commit to openedx/event-bus-kafka that referenced this issue Nov 21, 2022
Remove caching of `get_producer` since openedx-events will take care of
that for us.

This is part of openedx/openedx-events#87
timmc-edx added a commit that referenced this issue Nov 22, 2022
This defines an API for IDAs to use when producing events that allows the
choice of Event Bus implementation to be configured rather than written in
code. Implementations can document the appropriate setting for deployers
to use.

Caching the producer instance means that we automatically get a
long-lived instance without the implementation itself having to
perform caching itself.

This is part of #87
timmc-edx added a commit to openedx/event-bus-kafka that referenced this issue Nov 28, 2022
Remove caching of `get_producer` since openedx-events will take care of
that for us, and rename to `create_producer`.

This is part of openedx/openedx-events#87
Repository owner moved this from In Progress to Done in Arch-BOM Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
event-bus Work related to the Event Bus.
Projects
None yet
Development

No branches or pull requests

2 participants