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

Add artifact created event based on the outbox pattern #5274

Merged
merged 14 commits into from
Oct 10, 2024

Conversation

carlesarnal
Copy link
Member

@carlesarnal carlesarnal commented Sep 30, 2024

This PR introduces support for having Registry as a source for events. In the case of the SQL storage, an outbox table has been added and a row is created for each of the following events:

  • Group Created
  • Group Metadata Updated
  • Group Deleted
  • Artifact Created
  • Artifact Metadata Updated
  • Artifact Deleted
  • Artifact Version Created
  • Artifact Version Metadata Updated
  • Artifact Version Deleted
  • Global Rule Configured
  • Group Rule Configured
  • Artifact Rule Configured

Note that the event sending is done at the storage impl level on purpose, so we have access to the underlying transaction so we can benefit from the usual advantages of the outbox pattern in the SQL storage. We don't have now a way to intercept the transaction, so this was the only possible way.

@carlesarnal carlesarnal force-pushed the implement-registry-event-source branch 2 times, most recently from 5d4261b to 623ad42 Compare October 1, 2024 12:00
@carlesarnal carlesarnal force-pushed the implement-registry-event-source branch from 623ad42 to 128e13b Compare October 1, 2024 12:24
@carlesarnal carlesarnal force-pushed the implement-registry-event-source branch from dc83838 to b2aa62c Compare October 3, 2024 10:19
@carlesarnal carlesarnal marked this pull request as ready for review October 3, 2024 12:22
@carlesarnal carlesarnal force-pushed the implement-registry-event-source branch from 2e0695f to 835676a Compare October 4, 2024 08:47
Copy link
Member

@EricWittmann EricWittmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Nice testing. I had a couple minor comments.

@carlesarnal carlesarnal force-pushed the implement-registry-event-source branch from 94a179d to e95d66e Compare October 8, 2024 12:53
@carlesarnal carlesarnal force-pushed the implement-registry-event-source branch from afb1b36 to 1b7869e Compare October 9, 2024 10:06
@carlesarnal carlesarnal force-pushed the implement-registry-event-source branch from 1b7869e to dfe2dc9 Compare October 9, 2024 10:13
Copy link
Member

@EricWittmann EricWittmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as-is. However, question for you:

Do we ever want to offer the option of SQL storage variant + Kafka eventing without using Debezium?

@carlesarnal
Copy link
Member Author

LGTM as-is. However, question for you:

Do we ever want to offer the option of SQL storage variant + Kafka eventing without using Debezium?

Good question. If the Kafka cluster is there anyway, what would be the reason to not use Debezium? In the end, what we're doing is very basic CDC, so I don't see any benefits other than simplifying (a bit) the deployment model. For now, I'll merge as-is, we can revisit later if we think it makes sense.

@carlesarnal carlesarnal merged commit 5d756a9 into Apicurio:main Oct 10, 2024
20 checks passed
@carlesarnal carlesarnal deleted the implement-registry-event-source branch October 10, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants