FLUME-208: add support for "startFromEnd" param to tailDir

Review Request #1296 - Created Dec. 14, 2010 and submitted

Alex Baranau
old-flume
FLUME-208
flume
add support for "startFromEnd" param to tailDir
tests proper tailing while 
* adding new data into existing in dir files and 
* adding new files with data in a dir
  1. Alex,
    
    Looks pretty good, but there are a few nits and a few places where you need to make usage info consistent with code.
    
    Also, please update the documentation! Look here: ./src/docs/UserGuide/Appendix, and search for tailDir.
    
    Jon.
  2. nit: we've recently started to become picky about qualifier order.
    
    The order we prefer is "public protected private staitc final transient volatile" -- so this should be 'private final'
  3. I don't think this is an issue.
  4. Not sure if the "extra" stuff in flush() after the close will have negative affect.
    
    I think adding a function to Cursor that just closes the file and calling it would be straight foward and address the interruption.  
  5. slight tweaks to usage: (extra brackets, quotes not necessary on last arg, default arg should be false?)
    
    usage: tailDir(\"xx\"[, fileregex=\".*\"[, startFromEnd=false]])
    
    
    
    
  6. Usage says default is true. 
    Code says default false?
    
    Which is it? (I think I prefer false)
  7. if updated, I don't think you need quotes.
  8. nit: fix spaces
    
  9. 
      
Review request changed
  1. Alex, Looks good. Address the one thing below and this should be good.  
    
    Please upload a patch or a point me to a git branch so I can test the patch before committing!
    
    In the future, please respond to the comments in the review.  These are usually either 1) a simple 'ok' or 'yup' or 'k', 2) an explanation that address comment/question, or 3) a punt with a jira that will address the issue in the future.
  2. oh my bad, "xx" was just me not typing everything out.  having 'dir' or 'dirname' is more helpful.
    1. Alex, 
      
      Saw that this was changed in the patch you uploaded.  Looks good.  Please respond with a little comment here in the future as well so I can see an ack instead of searching for it.
      
      Thanks!
      Jon.
    2. OK! Looks like we set up the work process for future ;) Sorry for making you doing so many remarks about it.
  3. 
      
Loading...