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

Send Kafka-key til behov-rivers som aktiveres i parallell #797

Merged
merged 6 commits into from
Dec 10, 2024

Conversation

bjerga
Copy link
Contributor

@bjerga bjerga commented Nov 28, 2024

Kafka-keysene som sendes skal brukes i meldingen som behov-riverene sender som svar. Kort sagt så må alle behov-riverne som svarer en spesifikk service bruke den samme nøkkelen, slik at alle svarene går til den samme servicen. Nøkkelen må også være den samme for en forespørsel/sykmeldt på tvers av kall til API-et, slik at meldingene leses i samme rekkefølgen som de skrives.

@bjerga bjerga requested a review from a team as a code owner November 28, 2024 12:17
@bjerga
Copy link
Contributor Author

bjerga commented Nov 29, 2024

Testet OK.

@@ -192,6 +192,7 @@ private object Mock {
mapOf(
Key.ORGNR_UNDERENHETER to orgnr.toJson(Orgnr.serializer()),
),
svarKafkaKey = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Er den null fordi det ikke er noen partisjonering i test og den dermed ikke blir brukt til noe?
Eller er det noen tilfeller denne er null i prod også?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Den kan være null i prod også, foreløpig. Nå er det kun servicene med state som må bruke svarKafkaKey, mens de uten state ikke trenger det. Det er mulig at det er bedre om alle bruker det, men det tenkte jeg å se på etter denne PR-en.

Copy link
Contributor

@b162214 b162214 left a comment

Choose a reason for hiding this comment

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

Ser at det er noen merge konflikter. Ellers ser det greit ut 👍 🔑

Comment on lines +13 to +15
operator fun invoke(forespoerselId: UUID): KafkaKey = KafkaKey(forespoerselId.toString())

operator fun invoke(sykmeldtFnr: Fnr): KafkaKey = KafkaKey(sykmeldtFnr.verdi)
Copy link
Contributor

Choose a reason for hiding this comment

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

Snedig. Dette er for å slippe .toString() og .verdi rundt om kring i koden?

Copy link
Contributor Author

@bjerga bjerga Dec 10, 2024

Choose a reason for hiding this comment

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

Det er mer for å håndheve en streng type. Hvis man har lov til å bruke en hvilken som helst streng som Kafka-nøkkel så blir det ville vesten 🤠

(Jeg prøvde å lage to constructor, men det går ikke pga. konflikt under kompileringen til Java, derav invoke.)

@@ -54,6 +57,7 @@ class HentInntektRiver(
behovType = Key.BEHOV.krev(BehovType.HENT_INNTEKT, BehovType.serializer(), json),
transaksjonId = Key.KONTEKST_ID.les(UuidSerializer, json),
data = data,
svarKafkaKey = Key.SVAR_KAFKA_KEY.lesOrNull(KafkaKey.serializer(), data),
Copy link
Contributor

Choose a reason for hiding this comment

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

Er det sånn at vi i først neste omgang/PR faktisk skal publisere meldinger med denne nøkkelen, og at dette bare er foreberedelsene?

Jeg forsøkte å grave meg litt ned i riveren og kom til den linja her: ?.also { context.publish(null, it) } i OpenRiver.kt. Er det der vi etter hvert skal sende inn denne nøkkelen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Det stemmer at dette er forberedelser. null i context.publish(null, it) må byttes ut med en verdi. For alle som får svarKafkaKey, så skal vi bruke den verdien. For resterende så er ikke løsningen helt klar.

@bjerga bjerga merged commit b284aae into main Dec 10, 2024
74 checks passed
@bjerga bjerga deleted the dev/svar-kafka-key branch December 10, 2024 09:30
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.

3 participants