Skip to content

Conversation

yatharthranjan
Copy link
Member

Add an option to exclude certain topics when there are a lot of topics under the directory and will not be feasible to add each one as an output path in order to exclude a little number of topics.

Copy link
Contributor

@blootsvoets blootsvoets left a comment

Choose a reason for hiding this comment

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

LGTM, except for small comment. Should also be be fixed with #39, by adding a config file.

@Parameter(names = {"--max-files-per-topic"}, description = "Maximum number of records to process, per topic. Set below 1 to disable this option.")
public int maxFilesPerTopic = 0;

@Parameter(names = {"--exclude-topics"}, description = "Topics to exclude when processing the records.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Parameter name should be singular, since on the command line you specify one value per tag, e.g. --exclude-topic topicA --exclude-topic topicB.

Copy link
Member Author

Choose a reason for hiding this comment

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

It works as a comma-separated list. Is is preferred to use flag for each topic ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on JCommander docs, that is what I thought. I also think it is more clear from CLI point of view.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay I will update.

@yatharthranjan yatharthranjan merged commit 480e809 into dev Feb 15, 2019
@yatharthranjan yatharthranjan deleted the exclude_option branch February 15, 2019 16:38
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.

2 participants