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

Default Kafka topics for all integrations #958

Closed
3 tasks
simonhir opened this issue Nov 14, 2023 · 9 comments · Fixed by #1168
Closed
3 tasks

Default Kafka topics for all integrations #958

simonhir opened this issue Nov 14, 2023 · 9 comments · Fixed by #1168
Assignees
Labels
enhancement New feature or request

Comments

@simonhir
Copy link
Member

simonhir commented Nov 14, 2023

Currently the element templates for cosys, dms and other integrations require to add the Kafka topic for outgoing messages.
As these are different for all environments (topic suffix) this requires the process developers to define different configs for all envs and also they need to know what exact topic is the correct one for each of the integrations.

Goal

Business analysts should not know anything about the technical integration, but should rather select the business use case.

Describe solution

Migrations path

  1. provide new element template and motivate process designers to switch over
  2. if necessary, coordinate process migration for long-running process instances
  3. after a certain period of time, e.g. 1 month, switch to the new Kafka in the maintenance window

Acceptance criteria

  • The topic input in the element template is optional and if not set the default topic for the correlating env is used
  • The topics can be renamed with no effect on processes using only the default topics (needed for switch to new Kafka cluster)
  • DigiWF-Ops configured
@simonhir simonhir added enhancement New feature or request refinement labels Nov 14, 2023
@darenegade
Copy link
Member

@dominikhorn93 @lmoesle Habt ihr eine Idee, wie man das gut lösen kann?

@lmoesle
Copy link
Contributor

lmoesle commented Dec 1, 2023

Wir könnten auch die DIGIWF_ENV Property auslesen und bei Prozessstart als Prozessvariable setzen. Dann könnten wir in den Element Templates ein default Topic angeben (dwf-email-${digiwf-env}).
Der Vorteil wäre, dass die Prozessentwickler die Topics nicht mehr selbst setzen müssen und die Variable bei Bedarf auch für andere Funktionen nutzen könnten.

Wenn man das so machen würde, dann müsste man:

  • Die DIGIWF_ENV Property auslesen und beim Prozessstart als Prozessvariable setzen
  • Die Element Templates aller Integrationen anpassen, dass ein default Topic gesetzt wird

@simonhir
Copy link
Member Author

simonhir commented Dec 4, 2023

@lmoesle das Problem ist dann aber, dass wir damit die Topics nicht mehr einfach ändern können, weil ja dann jeder neu das Template auswählen müsste. Für den Schwenk auf das neue Cluster brauchen wir das aber.
Hab das gerade mal noch als Akzeptanz-Kriterium hinzugefügt hatte davor gefehlt.

@lmoesle
Copy link
Contributor

lmoesle commented Dec 4, 2023

Ein Feature unserer Platform ist es ja auch, dass jeder Prozessentwickler seine eigenen Integrationen anbinden kann. Dafür muss nur das Streaming Template mit dem richtigen Kafka Topic und Message Body aufgerufen.

@simonhir Die Variante 2 würde ich da dann eher nicht machen und Variante 1 bevorzugen. In diesem Schritt könnte man dann gleich den app_integration_name an die spring.cloud.function.definition der jeweiligen Integration anpassen. Aktuell sind die Namings da auch alle unterschiedlich. Wenn wir das angleichen, könnten wir den app_integration_name als type Header setzen und aus der Config des Connectors das passende Topic auswählen.

@LenaB34
Copy link

LenaB34 commented Dec 8, 2023

@lmoesle
Copy link
Contributor

lmoesle commented Jan 11, 2024

For #874 and #734 it would be a good idea to also add a header for the starting process key

@darenegade, @simonhir Aktuell ist es nicht ohne weiteres möglich den ProcessDefinitionKey an den Connector bzw. auch an die Integrationen weiterzugeben. Deswegen habe ich damals das Ticket #734 aufgemacht. Soll ich #734 dann gleich umsetzen?
Also, dass der ProcessDefinitionKey beim Prozessstart als Prozessvariable gesetzt wird und dadurch an den Connector aufruf weitergegeben wird?

@simonhir
Copy link
Member Author

Wir hatten das damals aufgenommen, weil wir dachten, dass das zeitlich weniger Aufwand bedeutet, da man da ja die gleichen Sachen anpassen müsste.
Aus meiner Sicht, müsste das nur spätestens im nächsten Sprint angepasst werden, weil das für die Paula-Integration benötigt wird.
Also je nachdem ob du das auch so siehst, dass das einfacher ist das gleich zu machen, würde ich sagen macht das schon Sinn.

@lmoesle
Copy link
Contributor

lmoesle commented Jan 11, 2024

Passt für mich, dann zieh ich #734 auch noch mit rein in den Sprint und setzte das im selben Branch um 👍🏻

@lmoesle
Copy link
Contributor

lmoesle commented Jan 12, 2024

Ich habe das Ticket auf Impediment verschoben, bis das Vorgehen bei #734 geklärt ist

@lmoesle lmoesle removed the blocked label Jan 22, 2024
lmoesle added a commit that referenced this issue Jan 29, 2024
* refactoring: rename application.properties to application.yml

* feature: introduce default destinations for integrations

* refactoring: remove digiwf-message-name from integration calls

* feature: adjust element templates, forms and bpmn process to default kafka topics

* chore: fix invalid element templates

* chore: remove app_message_name

* chore: remove app_message_name

* chore: remove app_message_name

* Revert "chore: remove app_message_name"

This reverts commit ecf9b68.

* Revert "chore: remove app_message_name"

This reverts commit c081f40.

* chore: replace app_message_name with app_integration_name

* chore: refactor configs

* fix: compiling error

* bugfix: fix incorrect function call

* chore: fix tests

* feature: use type for message correlation to avoid breaking changes

* feature: send digiwf.processdefinition header to integrations

* tmp: feature(#734): get processdefinition for integration call

* feature(#734): query root process instance id

* feature(#734): add feign error handling

* feature(#734): add tests

* feature(#734): add tests

* feature(#734): add tests

* feature(#734): add integration name to StreamingTemplateV01 and mark it as deprecated

* feature(#734): add integration name to StreamingTemplateV01 and mark it as deprecated

* feature(#734): add integration name to StreamingTemplateV01 and mark it as deprecated

* docs(#958): adjust documentation, element-templates and add connector documentation

* fix: address integration

* fix: processes

* Update EmailIntegration.json

* update element templates

---------

Co-authored-by: Lukas Mösle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants