docs: event bus related ADRs by jinder1s · Pull Request #40 · openedx/openedx-events

jinder1s · 2022-01-31T20:04:14Z

Some of the ADRs that came out of following issue discussions:
https://github.com/eduNEXT/openedx-events/issues/38
https://github.com/eduNEXT/openedx-events/issues/39
These adrs mostly cover issue 38, but do add some info from issue 39.

jinder1s · 2022-02-02T17:08:46Z

@dianakhuang Pinging you about this PR. I think it is ready for a final review.

dianakhuang

Looks good. Just had one last comment about some of the new things.

mariajgrimaldi · 2022-02-03T15:59:41Z

+
+      - The values can be: python primitives(int, str, dict, list ...), attrs decorated classes defined in openedx-events repository, or custom classes that have extensions defined in AvroAttrsBridge class.
+
+        - Ideally, the values are attrs decorated classes, other types are supported for convenience.


I'm realizing now that when instantiating OpenEdxPublicSignal we don't check for the data types. Maybe that's something we are interested in? So we are entirely sure the event it's using a data attr class

@mariajgrimaldi: I'm not clear on the validation proposal. I don't think we will be pursuing anything at this time, but if you see an important gap and you want to discuss more, feel free to explain.

robrap

Initial comments for our discussion.

jinder1s · 2022-02-07T19:19:24Z

Hi, I am going to do many changes to this PR based on review. Please hold off on further review for a bit.

jinder1s · 2022-02-07T21:23:43Z

@robrap This PR is ready for another review.

robrap

Thanks for the work on these @jinder1s.

The change log had various formatting bugs due to the inconsistent use of both `_` (underscore) and `-` (hyphen). The new format only uses hyphens. Added `=` for the top-level. Finally, reversed `-` and `~` to be more consistent with other docs, like the ADRs in this repo.

robrap · 2022-03-22T21:41:04Z

@dianakhuang: Can I get your review to get this over the line. Sorry - there is a lot going on in this PR. Let me know if anything should be split out.

Here is an overview of the changes:

Before making changes, I squashed all of Jinder's commits, which you had reviewed early on and I was on the verge of approving.
The original main point of this PR (and Jinder's commit) was introducing new ADRs 4 and 6 (was 5 when Jinder wrote it). The relatively minor updates I made to these two ADRs can be found in this commit: ac9ad17.
For consistency, in the commit 9abac30, I moved an ADR out from being a sub-directory of the Event Bus OEP. I then made minor tweaks to the original ADR in this separate commit: a3871eb
Note: the ADRs are marked Provisionsal, so hopefully we don't have to get to wrapped up in correctness while we are still working on all of this.

Additionally, I made the following minor changes (in separate commits):

Fixed the header formatting in the CHANGELOG.
Renamed howtos to how_tos.
Added the ADRs and How-Tos to the docs index page.

Separately, I asked tCRIL to publish these docs to readthedocs.

dianakhuang

I had some confusion about some of the phrasing in the first ADR.

robrap · 2022-03-24T00:28:42Z

@dianakhuang: Thanks. Ready for re-review.

dianakhuang

One typo to fix, but then I think you are good to merge.

Adds two new ADRs. 1. External event bus and Django Signal events: details the relationship between the new external event bus events and the existing internal Django Signal events. 2. Event schema serialization and evolution: details how an event schema will be used for event schema evolution. ARCHBOM-2010

Moving event schema ADR from sub-decision of the Event Bus ADR to the openedx-events repo, so it can live side-by-side with other related decisions.

The event schema ADR was first moved from the OEP repo without changes, and this commit makes some minor tweaks to make it work inside this repo. It also adds some notes about the CLoudEvents specification.

Renamed how_tos directory to align with OEP-19.

Make sections in the docs index for the ADRs (Decisions) and How-Tos.

robrap · 2022-03-24T16:19:17Z

@jinder1s: We finally merged it! 🎉

jinder1s · 2022-03-24T16:23:07Z

@robrap Hallelujah! It's Done! haha

robrap · 2022-03-24T16:27:11Z

Yes. Thanks for all your work on this @jinder1s. You are missed.

mariajgrimaldi · 2024-10-11T09:26:37Z

Hi, @robrap! All these ADRs have been provisional for some time now, should we change their status?

robrap · 2024-10-11T13:44:43Z

@mariajgrimaldi: Maybe https://github.com/openedx/openedx-events/blob/main/docs/decisions/0004-external-event-bus-and-django-signal-events.rst is the only one that wasn't updated? It can move to accepted. Thank you. Let me know if you will take care of that.

mariajgrimaldi · 2024-10-11T13:50:25Z

@robrap: I will! Thank you.

dianakhuang reviewed Feb 1, 2022

View reviewed changes

Comment thread docs/decisions/0004-event-bus-as-extension-of-django-signals.rst Outdated

Comment thread docs/decisions/0006-schema-implementation-decisions.rst Outdated

Comment thread docs/decisions/0006-schema-implementation-decisions.rst Outdated

jinder1s force-pushed the msingh/docs/adr/event_bus_overview branch from 11399cc to c418767 Compare February 2, 2022 16:54

jinder1s mentioned this pull request Feb 2, 2022

What to send over the wire (kafka)? #38

Closed

dianakhuang reviewed Feb 2, 2022

View reviewed changes

Comment thread docs/decisions/0004-event-bus-as-extension-of-django-signals.rst Outdated

mariajgrimaldi reviewed Feb 3, 2022

View reviewed changes

robrap reviewed Feb 3, 2022

View reviewed changes

Comment thread docs/decisions/0004-event-bus-as-extension-of-django-signals.rst Outdated

Comment thread docs/decisions/0004-event-bus-as-extension-of-django-signals.rst Outdated

jinder1s marked this pull request as draft February 7, 2022 19:18

jinder1s marked this pull request as ready for review February 7, 2022 21:23

robrap reviewed Feb 10, 2022

View reviewed changes

jinder1s force-pushed the msingh/docs/adr/event_bus_overview branch from 3d79a67 to 77c7c15 Compare February 11, 2022 16:17

robrap force-pushed the msingh/docs/adr/event_bus_overview branch from 54ef495 to ecdd612 Compare March 22, 2022 21:25

dianakhuang reviewed Mar 23, 2022

View reviewed changes

Comment thread docs/decisions/0004-external-event-bus-and-django-signal-events.rst Outdated

Comment thread docs/decisions/0004-external-event-bus-and-django-signal-events.rst Outdated

Comment thread docs/decisions/0004-external-event-bus-and-django-signal-events.rst Outdated

dianakhuang approved these changes Mar 24, 2022

View reviewed changes

Comment thread docs/decisions/0004-external-event-bus-and-django-signal-events.rst Outdated

dianakhuang approved these changes Mar 24, 2022

View reviewed changes

jinder1s and others added 5 commits March 24, 2022 12:14

docs: move event schema ADR

59b889e

Moving event schema ADR from sub-decision of the Event Bus ADR to the openedx-events repo, so it can live side-by-side with other related decisions.

docs: update moved event schema ADR

a8b5d89

The event schema ADR was first moved from the OEP repo without changes, and this commit makes some minor tweaks to make it work inside this repo. It also adds some notes about the CLoudEvents specification.

docs: rename how_tos directory

a92dcd6

Renamed how_tos directory to align with OEP-19.

docs: add ADRs and How-Tos to index

6dadd8b

Make sections in the docs index for the ADRs (Decisions) and How-Tos.

robrap force-pushed the msingh/docs/adr/event_bus_overview branch from c83aad7 to 6dadd8b Compare March 24, 2022 16:15

robrap merged commit 696d153 into main Mar 24, 2022

robrap deleted the msingh/docs/adr/event_bus_overview branch March 24, 2022 16:18

mariajgrimaldi mentioned this pull request Oct 11, 2024

docs: update repo purpose to status accepted #405

Merged


		- The values can be: python primitives(int, str, dict, list ...), attrs decorated classes defined in openedx-events repository, or custom classes that have extensions defined in AvroAttrsBridge class.

		- Ideally, the values are attrs decorated classes, other types are supported for convenience.

Uh oh!

Conversation

jinder1s commented Jan 31, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jinder1s commented Feb 2, 2022

Uh oh!

dianakhuang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mariajgrimaldi Feb 3, 2022

Choose a reason for hiding this comment

Uh oh!

robrap Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

robrap left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jinder1s commented Feb 7, 2022

Uh oh!

jinder1s commented Feb 7, 2022

Uh oh!

robrap left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robrap commented Mar 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dianakhuang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robrap commented Mar 24, 2022

Uh oh!

dianakhuang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robrap commented Mar 24, 2022

Uh oh!

jinder1s commented Mar 24, 2022

Uh oh!

robrap commented Mar 24, 2022

Uh oh!

mariajgrimaldi commented Oct 11, 2024

Uh oh!

robrap commented Oct 11, 2024

Uh oh!

mariajgrimaldi commented Oct 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

robrap commented Mar 22, 2022 •

edited

Loading