Thanks Ashish for the update. I will review the patch sometime soon, maybe early next week - sorry, swamped with other stuff. I recommend keeping the semantics the same as Avro Source (we can fix the dirs with sources issue later for both if that issue pops up).
Hari Thanks, Hari On Wednesday, July 17, 2013 at 10:35 PM, Ashish wrote: > Alex, > > Quick comment, I tried with directory name with spaces on Mac, it worked. > Don't have access to Windows box to test this. > Also, if this is the case, then Avro SSL implementation would also break. > It uses the same constructs. Do you want me to fix this? > > There is one more missing thing, keystore type is not supported. Avro > Source support this. I shall add support for this as well. > > Most of the stuff is ready, shall publish the patch again for review. > > Thanks for this review marathon :) > > > On Thu, Jul 18, 2013 at 12:37 AM, Alexander Alten-Lorenz < > [email protected] (mailto:[email protected])> wrote: > > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/12440/ > > > > flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSource.java<https://reviews.apache.org/r/12440/diff/1/?file=319796#file319796line125> > > (Diff > > revision 1) > > > > public class HTTPSource extends AbstractSource implements > > > > 125 > > > > keyStorePath = > > context.getString(HTTPSourceConfigurationConstants.SSL_KEYSTORE); > > > > I think this could break under some circumstances a windows integration, > > ex. when a path is like: > > C:\WINNT\PATH SOMEWHERE\DIRECTORY WITH SPACES\ > > > > You should workaround this. > > > > > > - Alexander Alten-Lorenz > > > > On July 10th, 2013, 7:40 a.m. UTC, Ashish Paliwal wrote: > > Review request for Flume. > > By Ashish Paliwal. > > > > *Updated July 10, 2013, 7:40 a.m.* > > *Bugs: * FLUME-2109 <https://issues.apache.org/jira/browse/FLUME-2109> > > *Repository: * flume-git > > Description > > > > Flume patch to support HTTPS support. Review comments incorporated. Now > > only one mode shall be enabled either normal HTTP or HTTPS. > > > > Testing > > > > Unit test case added to verify SSL functionality. > > > > Diffs > > > > - flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSource.java > > (c90f067) > > - > > flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSourceConfigurationConstants.java > > (f547e0f) > > - > > flume-ng-core/src/test/java/org/apache/flume/source/http/TestHTTPSource.java > > (8952db3) > > - flume-ng-doc/sphinx/FlumeUserGuide.rst (63cad21) > > > > View Diff <https://reviews.apache.org/r/12440/diff/> > > > > -- > thanks > ashish > > Blog: http://www.ashishpaliwal.com/blog > My Photo Galleries: http://www.pbase.com/ashishpaliwal > >
