a97001 / nestjs-rdkafka

A NestJS module wrapper for node-rdkafka.
MIT License
2 stars 3 forks source link

Are contribution accepted? #1

Open andreacioni opened 9 months ago

andreacioni commented 9 months ago

Hello @a97001 ,

I came across your repository and I found it really interesting as an alternative at the KafkaJs implementation used natively by NestJs. Then I saw the last commit and I saw it to be 4 years ago. I was wondering to make a contribution to move everything to a more recent version both for nestjs and node-rdkafka but, before start, I would like to know if may be interested to merge it once ready.

a97001 commented 9 months ago

Hello @a97001 ,

I came across your repository and I found it really interesting as an alternative at the KafkaJs implementation used natively by NestJs. Then I saw the last commit and I saw it to be 4 years ago. I was wondering to make a contribution to move everything to a more recent version both for nestjs and node-rdkafka but, before start, I would like to know if may be interested to merge it once ready.

Hi @andreacioni Thanks for your appreciation about this old repo. Yes, any contributions are welcome. 😀

andreacioni commented 8 months ago

I take a look at the code and I have many thoughts that I want to share with you before going further.

  1. Deprecate/Remove KafkaService: this is a quite radical change but needed to keep the library as clean as possible. That service encapsulate a lot of logic that it may not needed in most cases. For example:
    async onApplicationBootstrap(): Promise<void> {
    await this.createTopics();
    this.consumeMessages();
    }

    why should I be forced to create topics on startup? Sometimes application are not allowed to do that so doing this could lead this library to being unusable. Final users should encapsulate this kind of login in their application and don't expect this library to do that.

The exported provider should be three: rdkafka.Producer, rdkafka.KafkaConsumer, rdkafka.AdminClient. We can keep KafkaService as an internal provider to handle startup/shutdown task like connection/disconnection.

  1. Remove @Global annotation to module and add the possibility to define the module as global through KafkaConnectionOptions/KafkaConnectionAsyncOptions
  2. Implement forRoot and forRootAsync. The latter is the one that may be to configure the module with the support of other external services (e.g. ConfigService)
  3. Update nestjs version to latest
  4. Update node-rdkafka to latest version
  5. Abandon TravisCI in favor of Github Actions
  6. Add auto dependency update using Dependabot/Renovate
  7. Dependency cleanup, remove: mongodb, husky

What I've in mind is to move from version 1.0.5 to 1.1.0 in case we opt to deprecate KafkaService or to 2.0.0 in case we opt to remove it.

Please let me know your thoughts.