Skip to content

Specify additional output topics via default CLI#75

Merged
philipp94831 merged 3 commits intomasterfrom
feature/specify-multiple-output-topics
Aug 5, 2020
Merged

Specify additional output topics via default CLI#75
philipp94831 merged 3 commits intomasterfrom
feature/specify-multiple-output-topics

Conversation

@philipp94831
Copy link
Copy Markdown
Member

No description provided.

}

protected Optional<String> getOutputTopic(final String role) {
return Optional.ofNullable(this.extraOutputTopics.get(role));
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What do you think @VictorKuenstler @BJennWare : Should we return Optional or throw an exception if an output topic does not exist? i.e., delegate error handling to caller or handle it by default

Copy link
Copy Markdown
Contributor

@b-feldmann b-feldmann Aug 4, 2020

Choose a reason for hiding this comment

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

Can you think of a scenario for only outputting to a topic if configured? But having the freedom to do something like this might be worthwhile so I am fine with returning an optional.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's more for a case where it hasn't been configured. The question is if we would like to have a default error message then or let the user write one

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If it is only about an error message we can have a default one that includes the topic name and the role.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Better now?

Copy link
Copy Markdown
Contributor

@VictorKuenstler VictorKuenstler left a comment

Choose a reason for hiding this comment

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

LGTM

@philipp94831 philipp94831 merged commit 26f4348 into master Aug 5, 2020
@philipp94831 philipp94831 deleted the feature/specify-multiple-output-topics branch August 5, 2020 12:11
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