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

Is there any reason postfixId is forced? #9677

Closed
1 task done
twkim913 opened this issue May 27, 2022 · 2 comments · Fixed by #9681
Closed
1 task done

Is there any reason postfixId is forced? #9677

twkim913 opened this issue May 27, 2022 · 2 comments · Fixed by #9681
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@twkim913
Copy link

twkim913 commented May 27, 2022

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

const postfixId =
   this.getOptionsProp(this.options, 'postfixId') || '-server';

// append a unique id to the clientId and groupId
// so they don't collide with a microservices client
this.clientId =
  (clientOptions.clientId || KAFKA_DEFAULT_CLIENT) + postfixId;
this.groupId = (consumerOptions.groupId || KAFKA_DEFAULT_GROUP) + postfixId;

Is there any reason postfixId is forced?
(By forced I mean undefined || '-server' = '-server', '' || '-server' = '-server' )

Since I need to migrate old server and it's kafka Consumers to new nestJS server,
I need to preserve kafka consumer's old name.

Of course, I can do something like preserve old name delete-gift as with name 'delete' and postfixId '-gift',
but which is pretty implicit way, and I'd love to avoid it.

Describe the solution you'd like

Will it be possible to fix code to make it possible to set postfixId as '' ? (empty string)

Teachability, documentation, adoption, migration strategy

I don't know

What is the motivation / use case for changing the behavior?

This change will enable old kafka topic to be consumed by new nestJS server

@twkim913 twkim913 added needs triage This issue has not been looked into type: enhancement 🐺 labels May 27, 2022
@kamilmysliwiec
Copy link
Member

Will it be possible to fix code to make it possible to set postfixId as '' ? (empty string)

Would you like to create a PR for this? Simply changing from || to ?? should be sufficient

@twkim913
Copy link
Author

Since as @kamilmysliwiec clarified, there is no specific reason to force postfixId, and @micalevisk has done making PR for me.
I will close this issue, thank you to both of you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants