Migrate KafkaJS to ^2.0.0 #384

kalyvasio · 2024-06-20T14:17:16Z

Bumps KafkaJS to 2.0.0

It contains two major changes that affect us

Manage topics through admin client

KafkaJS contains some non-backwards compatible options in regards to managing topics via the admin client
We have changed the implementation to support the new array handling of topic and offsets

Default partitioner

Another change is the update of the DefaultPartitioner
By default the JavaCompatiblePartitioner is used but if not explicitly set it will log a warning
We are setting it by default (if not already set) here
If for some reason someone needs to use the old partitioner (LegacyPartitioner), it can be set through the .withKafkaOptions()
e.g .withKafka({ createPartitioner: Partitioners.LegacyPartitioner })

manoskouvarakis · 2024-06-26T14:10:57Z

src/initializers/kafka/kafka.ts

@@ -35,6 +34,7 @@ export default class OrkaKafka {
      authenticationTimeout
    });

+    if (options && !options.createPartitioner) options.createPartitioner = Partitioners.DefaultPartitioner;


Can we add the ability to define the partitioner in docs too?
https://github.com/Workable/orka/blob/master/docs/integrations/kafka.md

manoskouvarakis

🚀

nikostoulas · 2024-06-26T14:17:29Z

@kalyvasio Can we please also add the breaking changes here: https://workable.github.io/orka/#migrating-from-orka--3x-to-4x?

nikostoulas · 2024-06-26T14:19:00Z

src/initializers/kafka/kafka.ts

  const { username, password } = options.sasl || {};
-  if (username && password) return { sasl: options.sasl, ssl: options.ssl };
+  if (username && password) return { sasl: {...options.sasl }, ssl: options.ssl };


❓ Was this needed?

No it was a leftover from a reverted refactoring...Fixed in e1bc8c0

nikostoulas · 2024-06-26T14:21:30Z

src/utils.ts

@@ -25,7 +25,7 @@ export const nodeVersionGreaterThanEqual = (requestedVersion: string, version =
 };

 export function appendHeadersFromStore(
-  properties: { headers?: Record<string, string | Buffer | undefined> },
+  properties: { headers?: { [key: string]: Buffer | string | (Buffer | string)[] | undefined } },


❓ Was the previous complaining?

Yes but apparently for a different reason
Addressed here c057664

src/initializers/kafka/kafka.ts

antonakospanos · 2024-07-01T10:25:31Z

src/initializers/kafka/index.ts

@@ -39,7 +39,7 @@ function updateSend(kafka: Kafka, config: { traceHeaderName: string }) {
      const traceHeaderName = config.traceHeaderName.toLowerCase();
      appendHeadersFromStore(message, getRequestContext(), config);
      if (!message.key && message.headers[traceHeaderName]) {
-        message.key = message.headers[traceHeaderName];
+        message.key = message.headers[traceHeaderName] as Buffer | string | null;


Hmm these are the only values accepted by kafkajs typings for the message key.
I believe that all values are either treated as string or binary data.
Do you have a specific case you want me to take another look?

src/typings/kafka.d.ts

nikostoulas · 2024-07-02T15:43:10Z

@kalyvasio Can we please also add the breaking changes here: https://workable.github.io/orka/#migrating-from-orka--3x-to-4x?

FYI here

orka/docs/index.md

Line 77 in 915d882

    
           - Mongoose migrated to v7 which has breaking changes see [here](https://mongoosejs.com/docs/7.x/docs/migrating_to_7.html)

is the place to add the changes

kalyvasio · 2024-07-03T09:57:23Z

@kalyvasio Can we please also add the breaking changes here: https://workable.github.io/orka/#migrating-from-orka--3x-to-4x?

FYI here

orka/docs/index.md

Line 77 in 915d882

- Mongoose migrated to v7 which has breaking changes see [here](https://mongoosejs.com/docs/7.x/docs/migrating_to_7.html)

is the place to add the changes

I have updated the docs and I have added the usage for the Default Partitioner in the Kafka md in this commit
Please take a look and tell me if you want something different

nikostoulas · 2024-07-09T09:29:22Z

docs/index.md

@@ -75,3 +75,8 @@ ObjectId using new mongoose.
 - Changes in default config: istioTraceHeaders and headerPropagation are deprecated in favor of a generic propagatedHeaders
 - Methods kafka.send that was deprecated is removed. Use kafka.producer.send instead
 - Mongoose migrated to v7 which has breaking changes see [here](https://mongoosejs.com/docs/7.x/docs/migrating_to_7.html)
+
+### Migrating from orka  4.x to 5.x


@kalyvasio this is still migrating from 3.x to 4.x as we haven't gone out of beta yet. Use one line for the kafka changes

docs/integrations/kafka.md

Co-authored-by: Kyriakos Lesgidis <[email protected]>

Co-authored-by: Nikos Kostoulas <[email protected]>

kalyvasio self-assigned this Jun 20, 2024

kalyvasio force-pushed the migrate-kafkajs-to-2.0.0 branch 6 times, most recently from 83d44f1 to a7c9a54 Compare June 26, 2024 13:53

kalyvasio marked this pull request as ready for review June 26, 2024 14:04

kalyvasio requested review from jgerbe, klesgidis, kzacharakis, nikosd23, nikostoulas, stavros-wb and manoskouvarakis as code owners June 26, 2024 14:04

manoskouvarakis reviewed Jun 26, 2024

View reviewed changes

manoskouvarakis approved these changes Jun 26, 2024

View reviewed changes

nikostoulas reviewed Jun 26, 2024

View reviewed changes

klesgidis approved these changes Jun 26, 2024

View reviewed changes

src/initializers/kafka/kafka.ts Outdated Show resolved Hide resolved

src/initializers/kafka/kafka.ts Outdated Show resolved Hide resolved

kalyvasio force-pushed the migrate-kafkajs-to-2.0.0 branch from bf4b326 to e1bc8c0 Compare July 1, 2024 07:04

antonakospanos approved these changes Jul 1, 2024

View reviewed changes

antonakospanos requested a review from Ferrum-Citadel July 1, 2024 10:25

nikostoulas reviewed Jul 2, 2024

View reviewed changes

src/typings/kafka.d.ts Show resolved Hide resolved

kalyvasio force-pushed the migrate-kafkajs-to-2.0.0 branch from ae2f079 to c057664 Compare July 3, 2024 09:14

nikostoulas reviewed Jul 9, 2024

View reviewed changes

docs/integrations/kafka.md Outdated Show resolved Hide resolved

nikostoulas reviewed Jul 9, 2024

View reviewed changes

docs/integrations/kafka.md Outdated Show resolved Hide resolved

kalyvasio force-pushed the migrate-kafkajs-to-2.0.0 branch from edf800b to 5da9d39 Compare July 10, 2024 10:47

nikostoulas approved these changes Jul 10, 2024

View reviewed changes

kalyvasio and others added 5 commits July 10, 2024 17:15

Migrate KafkaJS to ^2.0.0

df7ccaf

Apply suggestions from code review

9d020af

Co-authored-by: Kyriakos Lesgidis <[email protected]>

fix typing for Message Headers

0e03fbc

Update docs

6334828

Apply suggestions from code review

dbf6cd5

Co-authored-by: Nikos Kostoulas <[email protected]>

kalyvasio force-pushed the migrate-kafkajs-to-2.0.0 branch from 5da9d39 to dbf6cd5 Compare July 10, 2024 14:15

kalyvasio merged commit 383dcba into master Jul 30, 2024
4 checks passed

kalyvasio deleted the migrate-kafkajs-to-2.0.0 branch July 30, 2024 10:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate KafkaJS to ^2.0.0 #384

Migrate KafkaJS to ^2.0.0 #384

kalyvasio commented Jun 20, 2024 •

edited

Loading

manoskouvarakis Jun 26, 2024

manoskouvarakis left a comment

nikostoulas commented Jun 26, 2024

nikostoulas Jun 26, 2024

kalyvasio Jul 1, 2024

nikostoulas Jun 26, 2024

kalyvasio Jul 1, 2024

antonakospanos Jul 1, 2024

kalyvasio Jul 1, 2024

nikostoulas commented Jul 2, 2024

kalyvasio commented Jul 3, 2024

nikostoulas Jul 9, 2024

	message.key = message.headers[traceHeaderName] as Buffer \| string \| null;
	message.key = message.headers[traceHeaderName] as Buffer \| string \| number \| null;

Migrate KafkaJS to ^2.0.0 #384

Migrate KafkaJS to ^2.0.0 #384

Conversation

kalyvasio commented Jun 20, 2024 • edited Loading

Manage topics through admin client

Default partitioner

Choose a reason for hiding this comment

manoskouvarakis left a comment

Choose a reason for hiding this comment

nikostoulas commented Jun 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikostoulas commented Jul 2, 2024

kalyvasio commented Jul 3, 2024

Choose a reason for hiding this comment

kalyvasio commented Jun 20, 2024 •

edited

Loading