[GitHub] nifi pull request #980: NIFI-1170 - Improved TailFile processor to support m...

[olegz <gi...@git.apache.org>]

Github user olegz commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/980#discussion_r78183927
  
    --- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TailFile.java ---
    @@ -117,31 +173,78 @@
                 .allowableValues(LOCATION_LOCAL, LOCATION_REMOTE)
                 .defaultValue(LOCATION_LOCAL.getValue())
                 .build();
    +
         static final PropertyDescriptor START_POSITION = new PropertyDescriptor.Builder()
                 .name("Initial Start Position")
    -            .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from the file, "
    +            .description("When the Processor first begins to tail data, this property specifies where the Processor should begin reading data. Once data has been ingested from a file, "
                         + "the Processor will continue from the last point from which it has received data.")
                 .allowableValues(START_BEGINNING_OF_TIME, START_CURRENT_FILE, START_CURRENT_TIME)
                 .defaultValue(START_CURRENT_FILE.getValue())
                 .required(true)
                 .build();
     
    +    static final PropertyDescriptor RECURSIVE = new PropertyDescriptor.Builder()
    +            .name("tailfile-recursive-lookup")
    +            .displayName("Recursive lookup")
    +            .description("When using Multiple files mode, this property defines if files must be listed recursively or not"
    +                    + " in the base directory.")
    +            .allowableValues("true", "false")
    +            .defaultValue("true")
    +            .required(true)
    +            .build();
    +
    +    static final PropertyDescriptor ROLLING_STRATEGY = new PropertyDescriptor.Builder()
    +            .name("tailfile-rolling-strategy")
    +            .displayName("Rolling Strategy")
    +            .description("Specifies if the files to tail have a fixed name or not.")
    +            .required(true)
    +            .allowableValues(FIXED_NAME, CHANGING_NAME)
    +            .defaultValue(FIXED_NAME.getValue())
    +            .build();
    +
    +    static final PropertyDescriptor LOOKUP_FREQUENCY = new PropertyDescriptor.Builder()
    +            .name("tailfile-lookup-frequency")
    +            .displayName("Lookup frequency")
    +            .description("Only used in Multiple files mode and Changing name rolling strategy, it specifies the minimum "
    +                    + "duration the processor will wait before listing again the files to tail.")
    +            .required(false)
    +            .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
    +            .defaultValue("10 minutes")
    --- End diff --
    
    Hmm, arbitrary timeouts always makes me worried unless there is some reasonable justification. For example, why 10 and not 20 min? In any event it seems like you want to give it a rather long time, unless overriden by user. If so wouldn't making it "infinite" accomplish same/similar goal - _will not re-list for 'n' time' where 'n' can be set by the user, otherwise its something like Long.MAX_VAL.
    
    This is by no means a directive, just a personal opinion, so would be curious to hear your thoughts.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---