-
Notifications
You must be signed in to change notification settings - Fork 0
feat(dev): PIN-7812 - dev configuration for notification services #297
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
base: main
Are you sure you want to change the base?
feat(dev): PIN-7812 - dev configuration for notification services #297
Conversation
commons/dev/configmaps/flyway-readmodel-agreement-configmap.yaml
Outdated
Show resolved
Hide resolved
CATALOG_TOPIC: "event-store.dev_catalog.events" | ||
DELEGATION_OUTBOUND_TOPIC: "outbound.dev_delegation.events" | ||
DELEGATION_TOPIC: "event-store.dev_delegation.events" | ||
EMAIL_SENDER_TOPIC: "dev_email-sender.emails" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New Kafka topic to be used for communication between the email-notification-dispatcher (that produces messages here when notifications are to be sent via email) and the email-sender (that consumes the message and actually sends the email via SES)
Should we do something to create the topic or is it automatically created on first connection?
TOPIC_STARTING_OFFSET: "latest" | ||
|
||
deployment: | ||
flywayInitContainer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set this to run migrations on the in-app notification DB, which are also run by the in-app-notification-manager
f3fed4c
to
87c8cfa
Compare
|
||
configmap: | ||
KAFKA_GROUP_ID: "{{.Values.namespace}}-email-notification-dispatcher" | ||
TOPIC_STARTING_OFFSET: "latest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set the dispatchers to latest
to avoid sending notifications for pre-existing events
|
||
configmap: | ||
KAFKA_GROUP_ID: "{{.Values.namespace}}-email-sender" | ||
TOPIC_STARTING_OFFSET: "earliest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's useful to set to earliest
, but this is a new topic so it's not an issue to reprocess older events
earliest
could be better if email-sender starts processing after some messages have already been created by the dispatcher
|
||
configmap: | ||
KAFKA_GROUP_ID: "{{.Values.namespace}}-notification-tenant-lifecycle-consumer" | ||
TOPIC_STARTING_OFFSET: "latest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This consumer and notification-user-lifecycle-consumer are set to latest
assuming that we'll use another process to import previous data (which is what we're assuming especially for the selfcare consumer)
@micdes-pagopa @borgesis95 Added a9f5820 to add two columns that were added to the readmodel in another PR but not here in the migrations |
Added another migration in 9621c21 which is required by pagopa/interop-be-monorepo#2434 |
On second thought, merging this isn't useful until we understand the changes needed for Selfcare integration, which is deeply connected to how dispatchers work. I'll mark this as draft and open a separate PR for the minimal changes included here which it makes sense to merge now |
c291e1e
to
e6f1aea
Compare
metadata_version INTEGER NOT NULL, | ||
user_id UUID NOT NULL, | ||
tenant_id UUID NOT NULL, | ||
user_roles VARCHAR[] NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we decided in previous similar cases, I edited this migration rather than adding a new one (on the other hand, I left the "grant access to dispatchers" as a separate migration for consistency with other readmodel schema migrations)
|
||
configmap: | ||
KAFKA_GROUP_ID: "{{.Values.namespace}}-notification-tenant-lifecycle-consumer" | ||
TOPIC_STARTING_OFFSET: "earliest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is set to earliest
so it creates default configurations for all existing tenants – event topics persist forever if I understood correctly from @galales, and the consumer handles both V1 and V2 events, so it should work.
Not sure whether it's useful to set to earliest
already on DEV or if we want to do it just on QA/PROD. On DEV we might not need it (we can just create some configs manually), so if it's an issue to set to earliest
(e.g. if it's resource-intensive for DEV to reprocess everything) we can set to latest
here.
|
||
configmap: | ||
KAFKA_GROUP_ID: "{{.Values.namespace}}-notification-user-lifecycle-consumer" | ||
TOPIC_STARTING_OFFSET: "latest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is set to latest
because the SC-Users topic has limited retention so we'll use another procedure to import past data
@micdes-pagopa I updated this PR and it's ready for review again (though it can't be merged and deployed until a few monorepo PRs, listed in the description, are merged) |
This PR adds the configuration to deploy on DEV all remaining services related to notifications