-
Notifications
You must be signed in to change notification settings - Fork 0
(chore): migrate to using confluent's kafka library #11
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?
Conversation
this.producer.flush(timeout, postFlushAction); | ||
} | ||
catch (err) { | ||
this.error('Producer encountered error while flusing events.', err); |
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.
There's a typo in the error message: flusing
should be corrected to flushing
.
this.error('Producer encountered error while flusing events.', err); | |
this.error('Producer encountered error while flushing events.', err); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
createPartitions(topic, totalPartitions, timeout, actionPostPartitionCreation) { | ||
try { | ||
this.adminClient.createPartitions(topic, totalPartitions, timeout, actionPostPartitionCreation); | ||
this.success(`Successfully created new topic partitons: topic=${topic}, totalParitions=${totalPartitions}`); |
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.
There's a typo in the success message: partitons
should be partitions
. Also, the same typo appears in the error message below this line.
this.success(`Successfully created new topic partitons: topic=${topic}, totalParitions=${totalPartitions}`); | |
this.success(`Successfully created new topic partitions: topic=${topic}, totalPartitions=${totalPartitions}`); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
this.success(`Successfully created new topic partitons: topic=${topic}, totalParitions=${totalPartitions}`); | ||
} | ||
catch (err) { | ||
this.error(`Encountered error while creating new partitions for topic: topic=${topic}, totalPartitons=${totalPartitions}`, err); |
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.
There's a typo in the error message: totalPartitons
should be spelled totalPartitions
for consistency with the parameter name and other occurrences in the code.
this.error(`Encountered error while creating new partitions for topic: topic=${topic}, totalPartitons=${totalPartitions}`, err); | |
this.error(`Encountered error while creating new partitions for topic: topic=${topic}, totalPartitions=${totalPartitions}`, err); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
'metadata.broker.list': 'localhost:9092', | ||
'socket.keepalive.enable': true, | ||
}, config, { 'client.id': clientId }); | ||
// commong topic configs defaults should go here. |
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.
There's a small typo in the comment on line 23 of client.ts
: commong
should be common
. This appears in the comment about topic config defaults.
// commong topic configs defaults should go here. | |
// common topic configs defaults should go here. |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
} | ||
/** | ||
* Connect to kafka server as admin. | ||
*/ |
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.
The JSDoc parameter format has a syntax error in the first parameter. The backtick is only present at the beginning of topic but missing at the end. For proper formatting, please ensure both opening and closing backticks are included: ``
topic` ``
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
*/ | ||
subscribe(topics: SubscribeTopicList): this; | ||
/** | ||
* Unsubscribe from all the subscribed topics.s |
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.
There's a small typo in the comment: topics.s
should be topics
. This appears in the JSDoc for the unsubscribe()
method.
* Unsubscribe from all the subscribed topics.s | |
* Unsubscribe from all the subscribed topics. |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
declare class KafkaAdmin extends Client { | ||
private adminClient; | ||
/** | ||
* Initialzes a KafkaAdmin client with config. |
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.
There's a small typo in the method documentation: Initialzes
should be corrected to Initializes
.
* Initialzes a KafkaAdmin client with config. | |
* Initializes a KafkaAdmin client with config. |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
private adminClient; | ||
/** | ||
* Initialzes a KafkaAdmin client with config. | ||
* Requires using connect() function after initalizing. |
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.
There's a small typo in the documentation comment: initalizing
should be spelled initializing
.
* Requires using connect() function after initalizing. | |
* Requires using connect() function after initializing. |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
No description provided.