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

feat: add annotations to subscriptions table #295

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yordis
Copy link
Contributor

@yordis yordis commented Jan 24, 2025

Context

The intent is to add information to build Operational dashboards based on such metadata. I need to contact the team owner of such a processor as soon as possible.

  • Should this be outside the Event Store?

This is a way to avoid extra infrastructure and tooling, and the code should be the source of truth rather than conforming to more complex organizational structures and more prominent companies' needs.

Here is an example:

defmodule ExampleHandler do
  use Commanded.Event.Handler,
    application: ExampleApp,
    name: "ExampleHandler",
    annotations: %{team: "billing"} # made up, pending to discuss in Commanded itself
end

Open Questions

  • Should the migration allow NULL values?
  • Should we do a merge when deploying or replace annotations when subscribing?

@yordis yordis force-pushed the add-annotations-to-event-handlers branch 7 times, most recently from 625f3dc to cae43a9 Compare January 24, 2025 01:42
@@ -0,0 +1,7 @@
-- Add annotations field to subscriptions table

ALTER TABLE subscriptions
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am confused about this file btw, do we even need these files since we use the SQL init module?

@yordis yordis force-pushed the add-annotations-to-event-handlers branch from cae43a9 to ece8b0c Compare January 24, 2025 01:47
@yordis yordis force-pushed the add-annotations-to-event-handlers branch from ece8b0c to f2080f6 Compare January 24, 2025 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant